diff --git a/server/src/__tests__/issues-goal-context-routes.test.ts b/server/src/__tests__/issues-goal-context-routes.test.ts index abc2978f4a1e..a7f4256e9c35 100644 --- a/server/src/__tests__/issues-goal-context-routes.test.ts +++ b/server/src/__tests__/issues-goal-context-routes.test.ts @@ -844,4 +844,102 @@ describe.sequential("issue goal context routes", () => { } }); }); + + // PEN-3114 door #15. Deliberately a SIBLING of the block above rather than a case + // inside it, because that block's local `beforeEach` denies `workspace_runtime:read` + // and this control is not gated on it. + // + // `defaultsJson` is masked unconditionally, so running it under the file's default + // blanket-allow models the *maximally entitled* caller — the hardest case for an + // unconditional mask to pass. Inside the block above it would have run as a denied + // viewer, where a future entitlement-gated implementation would also return nothing + // and the test would stay green without measuring the entitled path. That is the + // "test passes on unfixed code" shape this series keeps hitting, so the placement is + // load-bearing, not cosmetic. + // + // Why unconditional and not gated like the workspace runtime above: PEN-2852's + // `workspace_runtime:read` entitlement is scoped to workspace runtime config, and + // `defaultsJson` is plugin-manifest material — a different disclosure decision with a + // different audience. Gating it on that flag would hand plugin defaults to every + // holder of an unrelated entitlement. There is also no entitled consumer to serve: + // no UI reads `defaultsJson` off this projection at all. + describe("plugin defaultsJson masking (PEN-3114)", () => { + // The third open `Record` off this same + // response, and the one furthest from an operator's hands: + // `managedByPlugin.defaultsJson.settings` is copied verbatim out of a plugin + // manifest's `PluginManagedProjectDeclaration.settings` by + // `buildManagedProjectDefaults`. Same `assertIssueReadAllowed` gate, same + // `paperclipGetIssue` reader, one line below the `env: null` that proves this + // projection is a withholding boundary. + // + // The secret is again a DISTINCT invented value: reusing either constant above + // would let the PEN-2852 withholding on those exits satisfy the `not.toContain` + // assertion, and this test would pass against the unfixed projection. No workspace + // or execution workspace is mocked here either. + // + // Fixture values are invented; no real endpoint was called to produce them. + it("masks plugin-authored defaultsJson on the project from GET /issues/:id", async () => { + const PLUGIN_SETTINGS_SECRET = "invented-plugin-defaults-fixture-value"; + const managedByPlugin = { + id: "plugin-binding-1", + pluginId: "plugin-1", + pluginKey: "acme-tracker", + pluginDisplayName: "Acme Tracker", + resourceKind: "project", + resourceKey: "onboarding", + defaultsJson: { + projectKey: "onboarding", + displayName: "Onboarding", + description: null, + status: "in_progress", + color: null, + settings: { INTEGRATION_API_TOKEN: PLUGIN_SETTINGS_SECRET, region: "us-east-1" }, + }, + createdAt: new Date("2026-03-20T00:00:00Z"), + updatedAt: new Date("2026-03-20T00:00:00Z"), + }; + + mockIssueService.getById.mockResolvedValue({ ...legacyProjectLinkedIssue }); + mockProjectService.getById.mockResolvedValueOnce({ + ...(await mockProjectService.getById()), + managedByPlugin: structuredClone(managedByPlugin), + }); + + const res = await request(createApp()).get("/api/issues/11111111-1111-4111-8111-111111111111"); + + expect(res.status).toBe(200); + expect(JSON.stringify(res.body)).not.toContain(PLUGIN_SETTINGS_SECRET); + + const emitted = res.body.project.managedByPlugin; + // The closed, typed scalars survive — these are what addresses the binding, and + // `pluginDisplayName` / `pluginKey` / `resourceKey` are the three fields the UI + // actually reads off `managedByPlugin`. + expect(emitted.pluginKey).toBe("acme-tracker"); + expect(emitted.pluginDisplayName).toBe("Acme Tracker"); + expect(emitted.resourceKey).toBe("onboarding"); + + // Structure and every key name survive so the binding stays legible. + expect(Object.keys(emitted.defaultsJson)).toEqual([ + "projectKey", + "displayName", + "description", + "status", + "color", + "settings", + ]); + expect(Object.keys(emitted.defaultsJson.settings)).toEqual(["INTEGRATION_API_TOKEN", "region"]); + + // Values do not. Assert the credential leaf BY VALUE rather than by key + // presence — a regression passing `settings` through verbatim keeps the key set + // byte-identical, so a key-set assertion alone would stay green against it. + expect(emitted.defaultsJson.settings.INTEGRATION_API_TOKEN).toBe("***REDACTED***"); + // `region` is not credential-named and is masked anyway: this walk is + // mask-by-default, not a key-name denylist, and that is the property that makes + // it cover a manifest key nobody enumerated in advance. + expect(emitted.defaultsJson.settings.region).toBe("***REDACTED***"); + expect(emitted.defaultsJson.displayName).toBe("***REDACTED***"); + // `null` carries nothing and stays `null` rather than becoming the sentinel. + expect(emitted.defaultsJson.color).toBeNull(); + }); + }); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 02888ea37e87..e1e8a8b339bc 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -88,6 +88,7 @@ import { type IssueWakeDiagnosticsResponse, type IssueRelationIssueSummary, type IssueWatchdogDiscoveryKind, + type ProjectManagedByPlugin, type ProjectWorkspace, type SourceTrustMetadata, type SuccessfulRunHandoffState, @@ -7562,6 +7563,87 @@ export function issueRoutes( }; } + /** + * Mask the one open field on a project's plugin binding (PEN-3114, door #15 of the + * PEN-2370 series; ask 1 — names survive, values elided). + * + * `compactIssueProject` below is a projection, and the `env: null` line in it proves + * the author treated this response as a withholding boundary. `managedByPlugin` + * crossed it verbatim. `ProjectManagedByPlugin.defaultsJson` is an open + * `Record` (`packages/shared/src/types/project.ts`) over a `jsonb` + * column, and its `settings` leaf is copied straight out of a plugin manifest's + * `PluginManagedProjectDeclaration.settings` — "Optional plugin-specific defaults" + * (`packages/shared/src/types/plugin.ts`) — via `buildManagedProjectDefaults` + * (`services/projects.ts`). That block is authored by a plugin author rather than by + * a Paperclip operator, and integration config is a natural home for a credential. + * + * Enumerated rather than spread, so a field added to `ProjectManagedByPlugin` later + * has to be considered here instead of crossing silently — the same reason the + * enclosing function is a projection rather than a spread. + * + * ## Why delegate, and why to this walk + * + * `defaultsJson` goes through `maskWorkspaceRuntimeForRead` rather than a second walk + * written here: copying one is exactly how the array-shaped (#1574) and JSON-string + * (#1583) bypasses each shipped, so a finding against that walk should land here too. + * Its contract is the one ask 1 asks for — every value masked, every key name kept, + * anything that is not an object or array masked outright, depth-capped fail-closed. + * The last two matter more than they look: the column is `jsonb`, so the runtime value + * is arbitrary regardless of what the TypeScript type claims. + * + * Its `commands`/`services`/`jobs` identity carve-out is *inert* here — the platform + * writes `projectKey`/`displayName`/`description`/`status`/`color`/`settings` and none + * of those is one of those three array names. If a manifest ever did write a top-level + * `services` array, the carve-out would preserve only `id`/`name`/`label`/`title` + * strings on its entries, which that walk already discloses in the strictly more + * sensitive workspace-runtime position; the residual is bounded and no worse there. + * + * `withholdAgentConfigKeys` (#1581) was checked first and does not fit: it is keyed on + * the literal names `adapterConfig`/`runtimeConfig`, and it blanks its target to `{}`, + * which erases the key names ask 1 requires be kept. + * + * ## Why this is masked unconditionally, when the runtime exits above are gated + * + * The two workspace-runtime exits in this file now read + * `viewer.revealRuntimeConfig ? raw : …` (PEN-2852 / BLO-33407). This one deliberately + * does not, and the difference is the entitlement's scope rather than an oversight: + * `workspace_runtime:read` is defined over workspace runtime config. `defaultsJson` is + * plugin-manifest material with a different audience, so gating it on that flag would + * disclose plugin defaults to every holder of an unrelated entitlement — widening the + * grant while appearing to narrow it. + * + * Nor is there an entitled consumer to serve, which is what makes the gate valuable + * above: there, the runtime editors genuinely need raw values. Here no reader wants + * them (see below), so a gate would have an empty true-branch and the only effect of + * adding one would be the mis-scoping. If a consumer ever does need raw `defaultsJson`, + * it should arrive with its own entitlement rather than borrow this one. + * + * ## Why masking is safe here + * + * `defaultsJson` is retained to drive plugin reset/reconcile, and that path is + * write-only with respect to this response: it recomputes `defaults` from the manifest + * declaration and writes it to `pluginManagedResources.defaultsJson` + * (`services/projects.ts`), and `reset` updates the project row from the declaration + * too — neither ever reads this projection back. No UI reads `defaultsJson` at all: + * `ProjectDetail.tsx` reads `pluginDisplayName`, `pluginKey` and `resourceKey`, all of + * which survive untouched, and it reads them from `GET /projects/:id` rather than from + * this issue projection. + */ + function compactIssueManagedByPlugin(managed: ProjectManagedByPlugin | null | undefined) { + if (!managed) return null; + return { + id: managed.id, + pluginId: managed.pluginId, + pluginKey: managed.pluginKey, + pluginDisplayName: managed.pluginDisplayName, + resourceKind: managed.resourceKind, + resourceKey: managed.resourceKey, + defaultsJson: maskWorkspaceRuntimeForRead(managed.defaultsJson), + createdAt: managed.createdAt, + updatedAt: managed.updatedAt, + }; + } + function compactIssueProject( project: Awaited>["project"], viewer: WorkspaceRuntimeViewer, @@ -7590,7 +7672,7 @@ export function issueRoutes( compactIssueProjectWorkspace(workspace, viewer), ), primaryWorkspace: compactIssueProjectWorkspace(project.primaryWorkspace, viewer), - managedByPlugin: project.managedByPlugin ?? null, + managedByPlugin: compactIssueManagedByPlugin(project.managedByPlugin), taskCount: project.taskCount, budget: project.budget, archivedAt: project.archivedAt,