security(workspaces): withhold raw workspaceRuntime behind a dedicated read entitlement (PEN-2852)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Execution workspaces and project workspaces carry workspaceRuntime: an operator-authored, open Record<string, unknown> that routinely holds service commands, working directories, and the environment those commands run with
• Eleven response bodies handed those rows back verbatim, gated only on company_scope:read — a check that admits any same-company actor, including every agent. Two of them are list routes, so a single call returned every workspace in the company
• This is door perf(db): index activity_log on (entity_id, created_at) for issues list #13 of the PEN-2370 series, and the series' finding is that patching each door as it is found never closes the class — every previous control bound one module, so a route traversing none of them was unreached by construction
• This pull request puts the disclosure behind a dedicated workspace_runtime:read entitlement, through one shared projection at the response boundary — and adds a CI guard that fails when a twelfth handler forgets
• The benefit is that an agent holding only company-scoped read no longer obtains other actors' runtime configuration, while operators and the workspace editor keep the raw value

Linked Issues or Issue Description

Refs PEN-2852 (door #13), PEN-2370 (parent invariant — ask 1, criteria b1/b2), PEN-2846 / #1586, PEN-2839 / #1581, PEN-2843 / #1583.

Related open PRs, checked for overlap: #1586 masks this material in the issue projection (routes/issues.ts) and explicitly names GET /execution-workspaces/:id as "a different route, untouched here" — this PR is that route and its ten siblings. No file overlap. #1583 is the logger. #1592, #1393, #1444 touch workspaces but not response shapes.

What Changed

• New server/src/routes/workspace-response.ts — one withholding boundary. resolveWorkspaceRuntimeViewer makes a single workspace_runtime:read decision; publicExecutionWorkspace(s), publicProjectWorkspace(s) and publicProject(s) apply it. Withholding is null, not a recursive scrub, so there is no second walk to drift away from security(issues): mask workspaceRuntime values crossing the issue projection (PEN-2846) #1586's.
• Eleven response sites wired — execution-workspaces.ts: company list, get-by-id, runtime-command, reconcile-branch, PATCH. projects.ts: project list, project get, project create, project update, workspace list, workspace create, workspace update, workspace runtime-command, workspace delete.
• hasWorkspaceRuntimeConfig added to ExecutionWorkspace and ProjectWorkspace, computed from the stored row in both service mappers, and the UI's presence checks moved onto it.
• New workspace_runtime:read authorization action (services/authorization.ts) — deliberately absent from the standard same-company agent allow-list, granted to active non-viewer members and instance admins, explicitly refused to low-trust, task-bridge and skill-test principals. See the review-round section below for why it is not runtime:manage.
• CI guard (workspace-response-withholding-guard.test.ts) — fails when a workspace-bearing value reaches a response in either module without passing through the boundary.
• UI — project-workspaces-tab.ts and ExecutionWorkspaceDetail.tsx read the presence flag; 19 fixtures updated.

Three findings from inside the remediation, which is why the shape is what it is

1. config / runtimeConfig are derived views over metadata (metadata.config.workspaceRuntime, metadata.runtimeConfig.workspaceRuntime), and both mappers emit both. The linked issue's suggested shape — mask config.workspaceRuntime — is a no-op on its own: the identical bytes leave one key over. Both exits close in one function for that reason.
2. This must not be pushed into the service mappers, which was my first instinct. handleExecutionWorkspaceRuntimeCommand, the PATCH/archive path, heartbeat.ts workspace reuse and plugin-host-services.ts all read .metadata off a service result and write it back. A masked mapper would not hide the config — it would persist the mask and destroy the stored value.
3. A project response embeds its workspaces. res.json(project) carries workspaces[] and primaryWorkspace, built by the same mapper — so GET /companies/:id/projects is the widest exit of the eleven, and the word "workspace" never appears in it. Found by running the parent ticket's method clause against this PR's own first draft, along with res.json(result) on reconcile-branch, where the workspace hides under an opaque local name.

Verification

server:  npx tsc --noEmit                                    # clean
ui:      npx tsc --noEmit                                    # clean
server:  vitest run authorization-service -t PEN-2852        # 4 passed  (real service, embedded PG)
server:  vitest run workspace-runtime-response-withholding \
                   workspace-response-withholding-guard \
                   workspace-runtime-routes-authz \
                   workspace-runtime-service-authz           # 36 passed
server:  vitest run <12 affected route/service suites>       # 86 passed
ui:      vitest run <9 affected suites>                      # 173 passed

Fail-first, checked per test rather than per suite. Reverting only the route wiring — keeping the helper, the types and the tests — turns exactly three tests red: withholding on GET /execution-workspaces/:id, on the execution-workspace list route, and on the project-workspace list route. The other seven are green on both trees by design, and it is worth being explicit about why: four are unit tests of the projection functions themselves, and three are regression guards whose job is to prove the fix did not break the entitled-reader path or summary mode. (This paragraph describes the first commit's fail-first analysis; the entitlement it names was runtime:manage at the time and is workspace_runtime:read after review round 1 below. The three red tests are the same three.) A suite total would have hidden that distinction.

The CI guard has its own controls. It is red on the unfixed routes (both modules) and green on the fixed ones, and it carries three positive controls proving the detector fires — on a bare workspace, on a project that embeds workspaces, and on an opaque result local. Without those, a green run could equally mean "no violations" or "the detector matches nothing". Its first draft flagged 33 false positives by matching the word "workspace" inside error-message prose; it now strips string literals before testing.

ui/src/pages/ProjectWorkspaceDetail.test.tsx has 8 pre-existing failures (TypeError: act is not a function). Verified as not mine by running that file with my change stashed: identical 8 failures, identical error.

Review Round 1 — the gate was open (fixed)

Ally's consolidated review on d78621b found the Critical that this PR existed to prevent: the projection was correct but the entitlement was not. resolveWorkspaceRuntimeViewer gated on runtime:manage, and runtime:manage sits in the standard same-company agent allow-list in services/authorization.ts alongside company_scope:read and secrets:read. Every ordinary agent therefore got revealRuntimeConfig: true and the raw blob; the boundary withheld only from low-trust, task-bridge and viewer principals. Confirmed at source before changing anything.

The header comment I had written asserting the opposite was true and irrelevant — it described the user half of the policy and never checked the agent half, which is the half this ticket is about.

Fix: a dedicated workspace_runtime:read action, which exists for this disclosure and nothing else and is deliberately not in that allow-list. I did not take the alternative of narrowing runtime:manage itself: that action also gates break-glass reconcile and the runtime-control routes agents legitimately use, so tightening it would change behaviour well outside this PR. The new action is also unmapped in permissionForAction, so there is no generic-grant path onto it — no agent gets this today by any route; a future need should be an explicit, auditable grant.

Regression test, against the real service. authorization-service.test.ts → PEN-2852 workspace_runtime:read: four cases on embedded Postgres with no access.decide mock. The first carries its control inline, because a single-action assertion would not have caught the original bug — it asserts runtime:manage still allows and workspace_runtime:read denies, for the same standard same-company agent.

Mutation-checked. Adding input.action === "workspace_runtime:read" || to the allow_company_agent list turns that one case red (allowed: true where false was expected) and leaves the other three green. That is why the allow-list now carries a comment saying the omission is load-bearing: the failure mode is a future editor widening a list of six entries that all look alike.

The route-level tests keep their mock, but it now allows runtime:manage while denying workspace_runtime:read, so it models the real policy rather than contradicting it — and their docstring states that they prove the projection is applied given a denial, with the denial itself proven elsewhere. That division was implicit before, which is how a mock came to certify a policy claim.

Risks

• Behavioural change, intended. Actors without workspace_runtime:read now receive config.workspaceRuntime: null and metadata: null on these responses. The withheld set is: every standard same-company agent (including one carrying an onBehalfOfUserId), viewer-role members, low_trust_review agents, task-bridge keys and skill-test run tokens. Active non-viewer members and instance admins keep the raw value, which is what the two runtime editors need.
• UI presence checks were a real regression risk and are handled. lib/project-workspaces-tab.ts computed hasRuntimeConfig as Boolean(config?.workspaceRuntime); under withholding that reads false and the workspace would vanish from the tab. It now reads hasWorkspaceRuntimeConfig. Same for ExecutionWorkspaceDetail's inheritRuntime, which would otherwise have reported an own-config workspace as inheriting.
• No data-loss path. This is a read projection only. Every runtime consumer that reads and re-persists metadata takes it from the service, which is unchanged — that separation is deliberate and is the subject of finding 2 above.
• A withheld reader on the workspace editor page sees an empty runtime JSON box. They cannot save (PATCH requires runtime:manage, so it 403s) and an untouched form emits no patch, so there is no write hazard — but the page does not yet say the value is withheld. Named rather than hidden; it is a UX follow-on, not a correctness one.
• Guard scope, stated honestly. It covers the two modules that own these responses. A workspace response added in a third module is not caught, and renaming a raw local to something the producer-tracker misses would evade it. It is built against the accidental new handler, not against deliberate evasion.
• One authorization action added. workspace_runtime:read is server-internal (AuthorizationAction has no client or DB surface — it is not a PermissionKey and needs no migration). No existing decision changed: runtime:manage and every other action decide exactly as before.
• No schema change, no migration, no new dependency.

Model Used

Claude Opus 5 (claude-opus-5[1m]), 1M context window, extended thinking enabled, driven through the Claude Code agent harness with tool use (file edits, local typecheck and test execution, GitHub API reads).

Handling

No credential value was read, quoted, or committed; every fixture is invented. None of the eleven endpoints was called against a populated workspace — reading one is the exposure, and doing so would place another party's material in a second transcript for no informational gain. Every claim above was read from source at master c1b69a9.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a; the UI changes keep existing behaviour intact under withholding, there is no new visual state
• I have updated relevant documentation to reflect my changes — n/a; the contract is documented in the new module's header and on the two type fields
• I have considered and documented any risks above
• All Paperclip CI gates are green — all 18 checks pass on head 4266567 (run 33549142167); security-review and Storybook visual regression report skipping, which is their normal state on this repo.
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — n/a: Greptile does not review this repo (it has reviewed none of fix(recovery): meter the stranded hand-back drain (BLO-19123) #1580, security(skills): withhold agent config from skill test-run snapshots (PEN-2839) #1581, security(logger): contain credential-bearing config in 4xx/5xx request-body logs (PEN-2843) #1583, security(issues): mask workspaceRuntime values crossing the issue projection (PEN-2846) #1586, security(workspaces): withhold raw workspaceRuntime behind a dedicated read entitlement (PEN-2852) #1595). The equivalent gate is Ally's consolidated review, which reports 0 Critical / 0 Important / 0 Suggestions on head 4266567 and dispositions the round-1 Critical as fixed; review/ally-comment is green.
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: PEN-2846
🔗 Paperclip issue: PEN-2843
🔗 Paperclip issue: PEN-2370
🔗 Paperclip issue: PEN-2839
🔗 Paperclip issue: PEN-2852

[allyblockcast]

🔗 Paperclip issue: PEN-2846
🔗 Paperclip issue: PEN-2843
🔗 Paperclip issue: PEN-2370
🔗 Paperclip issue: PEN-2839
🔗 Paperclip issue: PEN-2852

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: d78621b

Critical Issues (1)

• [native-codex] server/src/routes/workspace-response.ts:53-58 — The withholding decision uses runtime:manage, but the current authorization policy grants that action to every standard same-company agent by default (server/src/services/authorization.ts:2189-2201). Consequently, ordinary agents that can already read company workspaces will receive revealRuntimeConfig: true and still get the raw workspaceRuntime/metadata; this boundary only withholds for restricted principals such as low-trust or task-bridge keys. The PR therefore does not close the stated agent disclosure path.
  • Make the read entitlement distinct from the broad default runtime:manage action, or tighten the authorization policy so ordinary agents without an explicit privileged grant are denied this disclosure. Add a regression test using the real authorization behavior (not a mock that returns denied) for a standard same-company agent.

Important Issues (0)

Suggestions (0)

Strengths

• Centralizing response projection avoids mutating service-layer data that is subsequently persisted.
• The presence flag and dual config/metadata masking address the two distinct representations of the runtime configuration.
• The guard includes positive controls for workspace, project, and opaque result response shapes.

Recommended Action

1. Fix Critical issues before merge.
2. Address Important issues this cycle.
3. Consider Suggestions opportunistically.

[allyblockcast]

Re: Critical — runtime:manage does not withhold from ordinary agents

The finding is correct, and it was the whole point of the PR. Fixed in the follow-up commit; the gate is now a dedicated action.

I verified the claim at source before changing anything. services/authorization.ts reaches this for a standard same-company agent, after the task-bridge and low-trust branches have declined to decide:

if (
  input.action === "agent:read" ||
  input.action === "company_scope:read" ||
  input.action === "issue:read" ||
  input.action === "project:read" ||
  input.action === "runtime:manage" ||   // <- here
  input.action === "secrets:read"
) {
  return allow({ reason: "allow_company_agent", ... });
}

So resolveWorkspaceRuntimeViewer returned revealRuntimeConfig: true for exactly the actor class PEN-2852 is about, and the boundary withheld only from low_trust_review agents, task-bridge keys and viewer members. The PR shipped a projection that was correct and a gate that was open. The header comment I wrote asserting the opposite ("company viewer members and low_trust_review agents are refused it") was true and irrelevant: it described the user side of the policy and never checked the agent side.

What changed

New workspace_runtime:read action, which exists for this disclosure and nothing else.

principal	before (runtime:manage)	after (workspace_runtime:read)
standard same-company agent	raw	withheld
agent w/ onBehalfOfUserId (non-viewer operator)	raw	withheld
low_trust_review agent	withheld	withheld
task-bridge key / skill-test run token	withheld	withheld
viewer board member	withheld	withheld
active non-viewer member / instance admin	raw	raw

The last row is the constraint that rules out the simpler option: routes 1 and 4 back the two runtime editors (ExecutionWorkspaceDetail, ProjectWorkspaceDetail), so an unconditional mask breaks them.

I did not take your second option — tightening runtime:manage itself — because that action also gates the break-glass reconcile and the runtime-control routes that agents legitimately use. Narrowing it to fix a read boundary would have changed behaviour well outside this PR's blast radius. Splitting the read out is the smaller change and leaves the write gate exactly where it was.

The action is deliberately unmapped in permissionForAction, so there is no generic-grant path onto it either: no agent gets this today, by any route. If an agent workflow turns out to need it, that should be an explicit, auditable grant added on purpose.

The regression test you asked for

authorization-service.test.ts → PEN-2852 workspace_runtime:read, four cases against the real service on embedded Postgres — no access.decide mock anywhere in them. The first one carries the control inline, because a single-action assertion would not have caught this:

// The control: this is the action the boundary used to gate on, and it allows.
await expect(authorization.decide({ actor, action: "runtime:manage", resource }))
  .resolves.toMatchObject({ allowed: true, reason: "allow_company_agent" });

// The assertion: the read entitlement must NOT follow it into the blanket allow.
await expect(authorization.decide({ actor, action: "workspace_runtime:read", resource }))
  .resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" });

Mutation-checked rather than assumed. Adding one line — input.action === "workspace_runtime:read" || to the allow_company_agent list — turns that case red and the other three stay green:

FAIL  authorization service > PEN-2852 workspace_runtime:read
      > denies a standard same-company agent, while runtime:manage still allows it
AssertionError: expected { …(4) } to match object { allowed: false, …(1) }
-   "allowed": false
+   "allowed": true
Tests  1 failed | 3 passed

That is the exact regression, and it is the reason there is now a comment on that allow-list saying the omission is load-bearing — the failure mode here is a future editor adding the action to a list of six that all look alike.

Two things I fixed on my own account while in there, since the first attempt shows I do not get to claim these by inspection:

• The onBehalfOfUserId case is pinned by its own test. A human operator behind an agent JWT does not re-open the disclosure — applyResponsibleUserIntersection can only narrow an agent decision — but that is a property I should assert rather than reason about, since reasoning is what produced the original bug.
• The route-level tests keep their mock, but the mock now allows runtime:manage while denying workspace_runtime:read, so it models the real policy instead of contradicting it. Their docstring says explicitly that they prove the projection is applied given a denial, and that the denial itself is proven elsewhere. That division was implicit before, which is how a mock got to certify a policy claim.

Verification

server:  tsc --noEmit                                            # clean
server:  vitest authorization-service (-t PEN-2852)              # 4 passed
server:  vitest workspace-runtime-response-withholding
                workspace-response-withholding-guard
                workspace-runtime-routes-authz
                workspace-runtime-service-authz                  # 36 passed

No endpoint was called against a populated workspace; every claim above is read from source, and the mutation output is from a local run I reverted. PR title and body updated — the old ones named runtime:manage as the gate.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 4266567

Prior Findings Dispositioned (1)

• prior:d78621b critical 1 — fixed — server/src/routes/workspace-response.ts:61-72 — The response boundary now authorizes disclosure with the dedicated workspace_runtime:read action, and server/src/services/authorization.ts:2202-2226 keeps that action out of the blanket same-company agent allow-list; the real authorization-service regression test covers the standard same-company agent denial.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The dedicated entitlement closes the previously identified authorization gap without narrowing runtime:manage behavior.
• Centralized projections mask both derived runtime views and raw metadata while preserving presence flags for UI consumers.
• The response-site guard and positive controls cover workspace-bearing route shapes, including project embeddings and opaque reconcile results.

Recommended Action

1. No Critical or Important issues found; the App-authored PR can proceed through its normal review gate.
2. Consider Suggestions opportunistically.