fix(authz): grant code access without repo memory ownership - #2802
Conversation
There was a problem hiding this comment.
LGTM: No major defects found; one minor code-scope spec contradiction remains below the requested major gate.
Note
Approved · head 43bc7a1 · 1 finding: 1 minor
| Severity | Finding | Where |
|---|---|---|
| minor | F1 Spec contradiction — authorization.md item 9: restricted code access still requires the repos ownership axis in prose | docs/reference/specs/authorization.md:18 |
F1 invariant: Touched specifications describing repository code authorization must describe compiled code access: explicit codeRepos overrides the repos fallback, human baselines and surface unions add code access, and delegation intersects code separately from repository memory ownership.
- authorization.md item 9 and its restrict validation criterion: restrict.repos contains acme/code, and a credential declares codeRepos: [acme/code] with repos absent. → The spec must describe this credential as admitted to code access without requiring a repos memory-ownership grant.
- authorization.md item 9: a credential declares repos: all with codeRepos: []. → The spec must describe code access as denied despite universal memory ownership; omitted codeRepos must still inherit repos.
- resident-repos.md item 26: the same code-only credential requests a restricted repository checkout. → The per-repository gate must be documented as checking compiled code access before executor selection, rather than the repos ownership axis.
- agent-general.md item 6: a code-only credential requests an issue write in its granted restricted repository. → The write gate must be documented as the resolved actor's compiled code access, not the requesting user's repos memory grant.
- routing-and-config.md item 4: repository authorization after resolution sees independent code and memory grants. → The spec must describe explicit codeRepos, the omitted-field repos fallback, and human baseline code access rather than requiring repos ownership.
- Human or surface grants and bound/delegated actors carry the new independent code declaration. → The descriptions must preserve additive human/surface code grants and separate code/memory delegation intersections; binding a person must not lend their ownership or code baseline to a credential.
Full review
F1 — Minor, high confidence: item 9 still says a restricted repo requires a repos grant, but codeRepos: [acme/code] now admits that repo without memory ownership; conversely, repos: all with codeRepos: [] denies code access. The same stale axis description remains in resident-repos item 26, agent-general item 6 and routing-and-config item 4. Update these statements to describe compiled code access, including the omitted-field fallback, rather than repository memory ownership.
43bc7a1 to
962324f
Compare
There was a problem hiding this comment.
LGTM: All six cases of 5423127447:F1 are fixed; no new consequential defects in the docs/comment delta.
Note
Approved · head 962324f · no findings
| Prior finding | Resolution | Evidence at this head |
|---|---|---|
| review:5423127447:F1 | fixed | Verified every prior case at 962324f against unchanged implementation: (1) authorization.md item9 and its restrict criterion describe codeRepos-only admission without repos memory ownership, matching parseGrantsConfig and POLICY repo:use; (2) explicit codeRepos: [] overrides repos: all for that entry, while omission inherits repos via repoAccessOf, and prose correctly allows other additive grants; (3) resident-repos26 names compiled code access before executor selection, matching dispatcher authorizeRepo at line2286 and refusal return before workspace attach; (4) agent-general6 names the resolved actor's code access, matching githubCapabilityFor.canWrite -> ConfigStore.canUseRepo and shared issue writeGate before API calls; (5) routing-and-config4 documents explicit codeRepos, legacy fallback and human code baselines without memory ownership; (6) grantsTable/grantsIn union human/surface code separately from repos, effectiveGrants/intersectGrants intersect code and memory independently, and resolveChatActor binding preserves credential grants with only self/asUser identity fields, never the person's code baseline or ownership. Corresponding src/config.ts and grants.ts comments, GitHub capability prose, docs/reference/authorization.md, configuration reference, migration/how-to and config.example.yaml agree with these paths. No executable implementation or test-file changes since 43bc7a1; existing independent credential assertions inspected, not rerun. |
Full review
5423127447:F1 is explicitly closed as fixed across all six cases; no new consequential defects were found in the docs/comment delta.
|
Warning Polylane could not verify the production impact of this pull request. Checked switchboard on coreplane-infra — 72h of invocations (111–4,812 req/h, errors flat outside one deploy burst) — and the resulting tree: codeRepos is additive and the changed repo:exec/repo row has no caller. The deployed grants document is not readable here. Also considered · 4 refuted
switchboard error templates over 48h (no authorization signature) · Internal error in Durable Object storage caused object to be reset; reference = 2gd0jqkb276gr1gj0o9rq2ho
Container error: Error: Runtime signalled the container to exit due to a new version rollout: 0
Container error: at SwitchboardServer.waitForPort (worker.js:21633:32)Full log (10 lines)Connection closed: this Durable Object instance is no longer active. Reconnect or retry the request.
Internal error in Durable Object storage caused object to be reset; reference = 2gd0jqkb276gr1gj0o9rq2ho
POST https://switchboard.coreplanelabs.dev/mcp
Network connection lost.
* * * * *
Container error: Error: Runtime signalled the container to exit due to a new version rollout: 0
The Workers runtime canceled this request because it detected that your Worker's code had hung and would never generate a response. Refer to: https://developers.cloudflare.com/workers/observability/errors/
GET https://switchboard.coreplanelabs.dev/healthz
Container error: at SwitchboardServer.waitForPort (worker.js:21633:32)
switchboard health check failed: 500Analysed against 7 cloud accounts and 1 repository
Polylane could not find the cloud resources this repository manages, so this review looked at the entire cloud account. Connect this repository to its resources and the next review will focus on exactly what this code deploys to. Polylane analysed Did this help? React 👍 or 👎 so the next review is sharper. |
Operators could not declare code access independently of repository memory ownership. The optional codeRepos field now grants code access alone; omitting it preserves existing repos entries. Agent grants remain explicit for credentials.
Why: Preserving existing operator code access with repos: all would also grant new memory ownership. These independent resource rights need separate declarations in the existing policy model.
Where to look
Feedback wanted: Review the new codeRepos declaration, delegation and full-authority checks, and code/memory compatibility. The core policy fix is already merged in #2799.
Risk: codeRepos requires the updated config reader. Omitting it preserves existing entries; human namespace baselines still apply. This PR includes no production config or deployment changes.
Verified: 1,001 scoped tests; root/scripts/Worker types, lint, format, specs, hygiene, consistency and coverage passed. Offline three-operator code/memory proof passed; all screenshot pixels unchanged.
Decisions (2)
Validation (4 criteria)
🤖 Generated with Claude Code