diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index aaf6063cb9..9f20cc21fe 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -552,6 +552,81 @@ describe("environment routes", () => { expect(res.body.envVars).toEqual({ MY_AGENT_TOOL_SETTING: "updated-value", EXTRA: "added" }); }); + it("scopes the envVars patch to an explicit companyId query for a multi-company actor", async () => { + const row = createPlatformSandboxEnvironment(); + mockEnvironmentService.getById.mockResolvedValue(row); + mockEnvironmentService.update.mockResolvedValue({ ...row, envVars: { A: "1" } }); + // No prior bindings and two memberships: neither inference path can + // pin a company, so the explicit query context must carry the save. + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + const app = createApp({ + ...ownerAdminActor, + companyIds: ["company-1", "company-2"], + memberships: [ + { companyId: "company-1", status: "active", membershipRole: "owner" }, + { companyId: "company-2", status: "active", membershipRole: "member" }, + ], + }); + + const res = await request(app) + .patch("/api/environments/env-managed-1?companyId=company-1") + .send({ envVars: { A: "1" } }); + + expect(res.status).toBe(200); + expect(mockSecretService.normalizeEnvBindingsForPersistence).toHaveBeenCalledWith( + "company-1", + { A: "1" }, + expect.anything(), + ); + }); + + it("falls back to the instance's only company when the actor's memberships cannot pin one", async () => { + const row = createPlatformSandboxEnvironment(); + mockEnvironmentService.getById.mockResolvedValue(row); + mockEnvironmentService.update.mockResolvedValue({ ...row, envVars: { A: "1" } }); + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + // An instance admin provisioned without membership rows — the + // owner-as-admin shape on managed stacks. + const app = createApp({ + ...ownerAdminActor, + companyIds: [], + memberships: [], + }); + + const res = await request(app) + .patch("/api/environments/env-managed-1") + .send({ envVars: { A: "1" } }); + + expect(res.status).toBe(200); + expect(mockSecretService.normalizeEnvBindingsForPersistence).toHaveBeenCalledWith( + "company-1", + { A: "1" }, + expect.anything(), + ); + }); + + it("still fails closed when no companyId context is resolvable on a multi-company instance", async () => { + mockEnvironmentService.getById.mockResolvedValue(createPlatformSandboxEnvironment()); + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1", "company-2"]); + const app = createApp({ + ...ownerAdminActor, + companyIds: ["company-1", "company-2"], + memberships: [ + { companyId: "company-1", status: "active", membershipRole: "owner" }, + { companyId: "company-2", status: "active", membershipRole: "member" }, + ], + }); + + const res = await request(app) + .patch("/api/environments/env-managed-1") + .send({ envVars: { A: "1" } }); + + expect(res.status).toBe(422); + expect(res.body.error).toContain("requires a companyId context"); + expect(mockEnvironmentService.update).not.toHaveBeenCalled(); + }); + it("rejects a patch that mixes envVars with any other field on the managed sandbox row", async () => { mockEnvironmentService.getById.mockResolvedValue(createPlatformSandboxEnvironment()); const app = createApp(ownerAdminActor); @@ -2497,7 +2572,9 @@ describe("environment routes", () => { expect(res.status).toBe(200); expect(res.body.ok).toBe(true); expect(mockProbeEnvironment).toHaveBeenCalledWith(expect.anything(), environment, { - companyId: null, + // The instance has exactly one company, so the secret-context fallback + // resolves it even though the actor carries no memberships. + companyId: "company-1", pluginWorkerManager: undefined, applyCustomImageTemplate: false, acquireSandboxRuntimeLease: false, @@ -2538,6 +2615,9 @@ describe("environment routes", () => { }; mockEnvironmentService.getById.mockResolvedValue(environment); mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + // A multi-company instance keeps the context genuinely ambiguous — a + // single-company instance would resolve via the instance fallback. + mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1", "company-2"]); const app = createApp({ type: "board", userId: "user-1", diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 7124c015a9..fbe07dc9f8 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -498,7 +498,8 @@ export function environmentRoutes( * Pick the company context used to create new secrets from raw-pasted * values, normalize env-var bindings, and resolve probe secrets. An * explicit route param / query wins, then the single company the - * environment's bindings already live in, then the actor's own company. + * environment's bindings already live in, then the actor's own company, + * then the instance's only company (when exactly one exists). * Bindings must never veto an explicit caller context: config-derived * bindings live in the company that owns each referenced secret (see * `replaceSecretRefsForInstanceTarget`), so an environment's bindings may @@ -526,6 +527,14 @@ export function environmentRoutes( if (req.actor.type === "board" && Array.isArray(req.actor.companyIds) && req.actor.companyIds.length === 1) { return req.actor.companyIds[0] ?? null; } + // Single-company instances have exactly one possible secret scope, so an + // actor whose memberships cannot pin a company (none, or several — e.g. an + // instance admin provisioned without a membership row) still resolves. + // Mirrors the fallback in `resolveCustomImageCompanyId`. + const instanceCompanyIds = await instanceSettings.listCompanyIds(); + if (instanceCompanyIds.length === 1 && instanceCompanyIds[0]) { + return instanceCompanyIds[0]; + } if (!options.required) return null; throw unprocessable( "Environment secret management requires a companyId context during the instance-scoped transition.", diff --git a/ui/src/api/environments.ts b/ui/src/api/environments.ts index 0c3873ca7a..5b1bc6e05f 100644 --- a/ui/src/api/environments.ts +++ b/ui/src/api/environments.ts @@ -54,7 +54,7 @@ export interface EnvironmentCustomImageRollbackResult { supersededTemplate: EnvironmentCustomImageTemplate; } -function customImageCompanyQuery(companyId: string): string { +function companyIdQuery(companyId: string): string { return `companyId=${encodeURIComponent(companyId)}`; } @@ -91,11 +91,21 @@ export const environmentsApi = { // write floor admits envVars-only patches there). envVars?: Environment["envVars"]; metadata?: Record | null; - }) => api.patch(`/environments/${environmentId}`, body), + // Secret-context company for env var / config writes. Without it the + // server can only infer a company from existing bindings or a + // single-membership actor, and fails closed otherwise — a fresh + // environment with no bindings needs the explicit context. + }, companyId?: string | null) => + api.patch( + companyId + ? `/environments/${environmentId}?${companyIdQuery(companyId)}` + : `/environments/${environmentId}`, + body, + ), probe: (environmentId: string, companyId?: string | null) => api.post( companyId - ? `/environments/${environmentId}/probe?${customImageCompanyQuery(companyId)}` + ? `/environments/${environmentId}/probe?${companyIdQuery(companyId)}` : `/environments/${environmentId}/probe`, {}, ), @@ -108,7 +118,7 @@ export const environmentsApi = { }) => api.post(`/companies/${companyId}/environments/probe-config`, body), customImageTemplate: (environmentId: string, companyId: string) => api.get( - `/environments/${environmentId}/custom-image-template?${customImageCompanyQuery(companyId)}`, + `/environments/${environmentId}/custom-image-template?${companyIdQuery(companyId)}`, ), startCustomImageSetupSession: ( environmentId: string, @@ -116,7 +126,7 @@ export const environmentsApi = { body: StartEnvironmentCustomImageSetupSession = {}, ) => api.post( - `/environments/${environmentId}/custom-image-setup-sessions?${customImageCompanyQuery(companyId)}`, + `/environments/${environmentId}/custom-image-setup-sessions?${companyIdQuery(companyId)}`, body, ), customImageSetupSession: (sessionId: string) => @@ -149,7 +159,7 @@ export const environmentsApi = { ), rollbackCustomImageTemplate: (environmentId: string, companyId: string) => api.post( - `/environments/${environmentId}/custom-image-template/rollback?${customImageCompanyQuery(companyId)}`, + `/environments/${environmentId}/custom-image-template/rollback?${companyIdQuery(companyId)}`, {}, ), disableCustomImageTemplate: ( @@ -158,6 +168,6 @@ export const environmentsApi = { options: { deleteProviderTemplate?: boolean } = {}, ) => api.delete( - `/environments/${environmentId}/custom-image-template?${customImageCompanyQuery(companyId)}&deleteProviderTemplate=${options.deleteProviderTemplate === true ? "true" : "false"}`, + `/environments/${environmentId}/custom-image-template?${companyIdQuery(companyId)}&deleteProviderTemplate=${options.deleteProviderTemplate === true ? "true" : "false"}`, ), }; diff --git a/ui/src/pages/CompanyEnvironments.test.tsx b/ui/src/pages/CompanyEnvironments.test.tsx index b0d5c10ff9..cd8a7e52b0 100644 --- a/ui/src/pages/CompanyEnvironments.test.tsx +++ b/ui/src/pages/CompanyEnvironments.test.tsx @@ -685,6 +685,9 @@ describe("CompanyEnvironments — test provider button", () => { driver: "sandbox", envVars: { API_TOKEN: { type: "plain", value: "draft-token" } }, }), + // The secret-context company must ride along so the server can scope + // bindings even when the environment has none yet. + "company-1", ); expect(getEnvironmentFormPage()).toBeNull(); }); diff --git a/ui/src/pages/CompanyEnvironments.tsx b/ui/src/pages/CompanyEnvironments.tsx index 2c337bf188..f974caddf8 100644 --- a/ui/src/pages/CompanyEnvironments.tsx +++ b/ui/src/pages/CompanyEnvironments.tsx @@ -1258,7 +1258,7 @@ export function CompanyEnvironments({ mode = "list" }: CompanyEnvironmentsProps) const managedEnvironmentEnvVarsMutation = useMutation({ mutationFn: async (envVars: EnvironmentFormState["envVars"]) => { if (!editingEnvironmentId) throw new Error("No environment selected"); - return await environmentsApi.update(editingEnvironmentId, { envVars }); + return await environmentsApi.update(editingEnvironmentId, { envVars }, selectedCompanyId); }, onSuccess: async (environment) => { if (selectedCompanyId) { @@ -1291,7 +1291,7 @@ export function CompanyEnvironments({ mode = "list" }: CompanyEnvironmentsProps) const body = buildEnvironmentPayload(form); if (editingEnvironmentId) { - return await environmentsApi.update(editingEnvironmentId, body); + return await environmentsApi.update(editingEnvironmentId, body, selectedCompanyId); } if (!selectedCompanyId) throw new Error("Select a company to create environments");