diff --git a/workspace-server/internal/handlers/registry.go b/workspace-server/internal/handlers/registry.go index 7278b4dc..8bd694b8 100644 --- a/workspace-server/internal/handlers/registry.go +++ b/workspace-server/internal/handlers/registry.go @@ -341,6 +341,27 @@ func (h *RegistryHandler) Register(c *gin.Context) { log.Printf("Registry: token existence check failed for %s: %v", payload.ID, hasLiveErr) } + // RFC #2312 PR-F: return the workspace's platform_inbound_secret so SaaS + // workspaces (which have no persistent /configs volume across container + // restarts) can re-populate /configs/.platform_inbound_secret on every + // register call. Docker-mode workspaces also receive it — the workspace- + // side write is idempotent (same value every call until a future + // rotation flow lands), so the duplication is harmless. + // + // NOT gated by hasLive: the inbound secret is minted at workspace + // creation in workspace_provision.go (PR-A), independent of the + // outbound auth_token's "issue once" lifecycle. Returning it here is + // the only delivery path for SaaS, where the platform's CP provisioner + // has no volume to write into. + if secret, secretErr := wsauth.ReadPlatformInboundSecret(ctx, db.DB, payload.ID); secretErr == nil { + response["platform_inbound_secret"] = secret + } else if !errors.Is(secretErr, wsauth.ErrNoInboundSecret) { + // ErrNoInboundSecret is the expected case for legacy workspaces + // that predate migration 044 — quiet. Other errors (DB hiccup, etc.) + // log loud so ops notices. + log.Printf("Registry: read platform_inbound_secret for %s failed: %v", payload.ID, secretErr) + } + c.JSON(http.StatusOK, response) } diff --git a/workspace-server/internal/handlers/registry_test.go b/workspace-server/internal/handlers/registry_test.go index 9f33b140..645dd924 100644 --- a/workspace-server/internal/handlers/registry_test.go +++ b/workspace-server/internal/handlers/registry_test.go @@ -889,6 +889,129 @@ func TestRegister_C18_BootstrapAllowedNoTokens(t *testing.T) { } } +// TestRegister_ReturnsPlatformInboundSecret_RFC2312_PRF verifies that +// /registry/register includes the workspace's platform_inbound_secret +// in the response body when one is on file. This is the SaaS delivery +// path: SaaS workspaces have no persistent /configs volume, so they +// re-fetch the secret on every register call (idempotent in Docker mode +// where the provisioner already wrote the same value to the volume at +// workspace creation). +func TestRegister_ReturnsPlatformInboundSecret_RFC2312_PRF(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewRegistryHandler(broadcaster) + + const wsID = "00000000-0000-0000-0000-000000002312" + const inboundSecret = "the-platform-inbound-secret-value" + + // requireWorkspaceToken — bootstrap allowed (no live tokens). + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + + // Workspace upsert. + mock.ExpectExec("INSERT INTO workspaces"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectQuery("SELECT url FROM workspaces WHERE id"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://localhost:9100")) + mock.ExpectExec("INSERT INTO structure_events"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + // Phase 30.1 token issuance — first-register path. + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + mock.ExpectExec("INSERT INTO workspace_auth_tokens"). + WillReturnResult(sqlmock.NewResult(1, 1)) + + // RFC #2312 PR-F: ReadPlatformInboundSecret query — returns the value + // the provisioner stored at workspace creation. The handler MUST + // include this in the response body so the workspace can persist it + // to /configs/.platform_inbound_secret. + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow(inboundSecret)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/registry/register", + bytes.NewBufferString(`{"id":"`+wsID+`","url":"http://localhost:9100","agent_card":{"name":"x"}}`)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Register(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("parse response: %v", err) + } + got, ok := resp["platform_inbound_secret"].(string) + if !ok { + t.Fatalf("expected platform_inbound_secret in response, got: %v", resp) + } + if got != inboundSecret { + t.Errorf("secret mismatch: got %q, want %q", got, inboundSecret) + } + // auth_token should also be present (first-register path). + if resp["auth_token"] == nil { + t.Error("expected auth_token in response (first-register path)") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// TestRegister_NoInboundSecret_OmitsField verifies that legacy workspaces +// that predate migration 044 (NULL platform_inbound_secret column) still +// get a successful registration — the field is just omitted from the +// response. The Register handler logs the absence quietly. +func TestRegister_NoInboundSecret_OmitsField(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewRegistryHandler(broadcaster) + + const wsID = "00000000-0000-0000-0000-000000002312" + + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + mock.ExpectExec("INSERT INTO workspaces").WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectQuery("SELECT url FROM workspaces WHERE id"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://localhost:9100")) + mock.ExpectExec("INSERT INTO structure_events").WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + mock.ExpectExec("INSERT INTO workspace_auth_tokens").WillReturnResult(sqlmock.NewResult(1, 1)) + // NULL secret — legacy workspace. + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow(nil)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/registry/register", + bytes.NewBufferString(`{"id":"`+wsID+`","url":"http://localhost:9100","agent_card":{"name":"x"}}`)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Register(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200 even without inbound secret, got %d", w.Code) + } + var resp map[string]interface{} + _ = json.Unmarshal(w.Body.Bytes(), &resp) + if _, present := resp["platform_inbound_secret"]; present { + t.Errorf("expected platform_inbound_secret to be ABSENT for legacy workspace, got: %v", resp["platform_inbound_secret"]) + } +} + // TestRegister_C18_HijackBlockedNoBearer verifies the C18 attack is blocked: // when a workspace already has a live token, /register without a bearer → 401. func TestRegister_C18_HijackBlockedNoBearer(t *testing.T) { diff --git a/workspace/main.py b/workspace/main.py index 68394b51..79540c17 100644 --- a/workspace/main.py +++ b/workspace/main.py @@ -310,6 +310,17 @@ async def main(): # pragma: no cover from platform_auth import save_token save_token(tok) print(f"Saved workspace auth token (prefix={tok[:8]}…)") + # RFC #2312 PR-F: persist platform_inbound_secret if the + # platform supplied one. Idempotent — writing the same + # value over an existing file is harmless. Required for + # SaaS where there's no persistent /configs volume; on + # Docker mode it overwrites the value the provisioner + # already wrote at workspace creation. + inbound = body.get("platform_inbound_secret") + if inbound: + from platform_inbound_auth import save_inbound_secret + save_inbound_secret(inbound) + print(f"Saved platform_inbound_secret (prefix={inbound[:8]}…)") except Exception as parse_exc: print(f"Warning: couldn't parse register response for token: {parse_exc}") except Exception as e: diff --git a/workspace/platform_inbound_auth.py b/workspace/platform_inbound_auth.py index dc3b8317..0a8dd8ee 100644 --- a/workspace/platform_inbound_auth.py +++ b/workspace/platform_inbound_auth.py @@ -72,6 +72,47 @@ def reset_cache() -> None: _cached_secret = None +def save_inbound_secret(secret: str) -> None: + """Persist a freshly-received platform_inbound_secret to disk. + + Called from the /registry/register response handler when the platform + returns a `platform_inbound_secret` field. Mirrors platform_auth.save_token's + pattern: 0600 file in CONFIGS_DIR, atomic write via tmp + rename so a + concurrent reader never sees a partial file. + + Idempotent: writing the same value over an existing file is a no-op + from the workspace's perspective. Resets the in-process cache so the + next get_inbound_secret() returns the freshly-written value (matters + when a future rotation flow lands and the platform sends a different + secret on a subsequent register call). + """ + global _cached_secret + if not secret: + return + path = _secret_file() + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(path.suffix + ".tmp") + try: + # Open with 0600 from the start so a concurrent reader can never + # see a 0644-default fd before the chmod. mode= is honored by + # os.open underneath; pathlib.write_text does not expose it. + fd = os.open(str(tmp), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w") as f: + f.write(secret) + os.replace(str(tmp), str(path)) + # Race-safe in-process cache update: clear first, then let next + # caller re-read disk. Avoids the "stored new, cache still has + # old" window if get_inbound_secret races with this write. + _cached_secret = None + except OSError as exc: + logger.warning("platform_inbound_auth: save %s failed: %s", path, exc) + # Best-effort cleanup of the tmp file. + try: + os.unlink(str(tmp)) + except OSError as cleanup_exc: + logger.debug("platform_inbound_auth: unlink tmp %s failed: %s", tmp, cleanup_exc) + + def inbound_authorized(expected_secret: str | None, auth_header: str) -> bool: """Return True iff a /internal/* request should be served. diff --git a/workspace/tests/test_platform_inbound_auth.py b/workspace/tests/test_platform_inbound_auth.py index 81c1abed..d8801051 100644 --- a/workspace/tests/test_platform_inbound_auth.py +++ b/workspace/tests/test_platform_inbound_auth.py @@ -118,3 +118,57 @@ def test_end_to_end_file_to_authorized(configs_dir: Path): secret = get_inbound_secret() assert inbound_authorized(secret, "Bearer e2e-secret") is True assert inbound_authorized(secret, "Bearer not-this") is False + + +# ───────────── save_inbound_secret (RFC #2312 PR-F) ───────────── + +from platform_inbound_auth import save_inbound_secret + + +def test_save_inbound_secret_writes_file(configs_dir: Path): + save_inbound_secret("fresh-secret-from-register") + assert (configs_dir / ".platform_inbound_secret").read_text() == "fresh-secret-from-register" + + +def test_save_inbound_secret_writes_0600_mode(configs_dir: Path): + """File mode MUST be 0600. Anything else lets co-resident processes + read the bearer the platform uses to call /internal/* endpoints.""" + save_inbound_secret("mode-test") + mode = (configs_dir / ".platform_inbound_secret").stat().st_mode & 0o777 + assert mode == 0o600, f"expected 0600, got {oct(mode)}" + + +def test_save_inbound_secret_overwrites_existing(configs_dir: Path): + """Idempotent — saving over an existing file replaces the content + cleanly (atomic via tmp + rename).""" + (configs_dir / ".platform_inbound_secret").write_text("old-value") + save_inbound_secret("new-value") + assert (configs_dir / ".platform_inbound_secret").read_text() == "new-value" + + +def test_save_inbound_secret_invalidates_cache(configs_dir: Path): + """After saving, the next get_inbound_secret() must return the NEW + value, not the cached old one. Otherwise rotation would be silently + broken once we ever rotate.""" + (configs_dir / ".platform_inbound_secret").write_text("v1") + assert get_inbound_secret() == "v1" # primes cache + save_inbound_secret("v2") + assert get_inbound_secret() == "v2" # cache invalidated, re-reads + + +def test_save_inbound_secret_empty_is_noop(configs_dir: Path): + """An empty secret string is treated as 'platform didn't return one' + and ignored — the existing file (if any) stays untouched.""" + (configs_dir / ".platform_inbound_secret").write_text("existing") + save_inbound_secret("") + assert (configs_dir / ".platform_inbound_secret").read_text() == "existing" + + +def test_save_inbound_secret_creates_parent_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """If CONFIGS_DIR doesn't exist yet (very first boot), save_inbound_secret + creates it rather than KeyError-ing.""" + nonexistent = tmp_path / "fresh" / "configs" + monkeypatch.setenv("CONFIGS_DIR", str(nonexistent)) + platform_inbound_auth.reset_cache() + save_inbound_secret("bootstrap-value") + assert (nonexistent / ".platform_inbound_secret").read_text() == "bootstrap-value"