audit fixes (P9): narrow the agent push permissions and correct the assistant tooling config - #2612
Conversation
…M21) Defect: `.claude/settings.json` allowed `Bash(git push:*)`, and Claude Code Bash rules are prefix matches, so every push spelling the deny list did not name — `origin --force <b>`, `origin +<b>`, `-u origin main`, `--delete`, `--mirror`, `--no-verify` (which also skips guard-push.mjs) — ran with no confirmation. Trigger: an agent session composing any of those spellings, e.g. `git push origin +claude/x` after a rebase or `git push -u origin main` by mistake. Fix: the allow list now names only plain `git push`, `git push origin HEAD`, and `git push -u origin claude/<branch>`; every other spelling falls back to the default ask. The deny list spells out force, refspec-force, delete, mirror, hook-skipping, and main/master-target shapes in each spelling. Proof: `tests/claude-code-settings.test.ts` "git push tightening" — 28 dangerous spellings must match a deny rule and no allow rule, five prompt-only spellings must match no allow rule, and the three safe spellings must stay allowed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
…* (M35) Defect: the RAG module moved into `src/lib/rag/`, but `rag-retrieval-reviewer` still scoped `src/lib/rag*.ts`, `src/lib/rag-answer*.ts` and `src/lib/rag-eval-cases.ts`, and `clinical-governance-reviewer` still scoped `src/lib/rag-source-block.ts`, `rag-quote-verification.ts` and `rag-answer-support.ts`. None matched a file, and a `src/lib/rag*.ts` glob does not descend into the directory, so the two reviewers meant to cover the protected tree were told to look elsewhere. Trigger: invoking either subagent on a change under `src/lib/rag/**`, or relying on the agent description for auto-selection. Fix: both scopes now name `src/lib/rag/**` (with the grounded-evidence files called out for the governance reviewer) and the description says `src/lib/rag/**`. Agent prose only; no RAG surface changes. Proof: new `tests/agent-scopes.test.ts` brace-expands every backticked scope entry in `.claude/agents/*.md` against the tree and requires the two RAG-facing reviewers to scope `src/lib/rag/**`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
Defect: `.cursor/mcp.json` pointed the Supabase MCP at the live clinical project with `read_only=true` but no `features=` restriction, so a Cursor session got the server's default read-only tool groups — database (SQL, tables) and debugging (logs, advisors) — held back by prose only. The Claude (`.mcp.json`) and Codex (`.codex/config.toml`) entries pin `features=docs,development`. Trigger: any Cursor session with the project MCP loaded. Fix: add `&features=docs%2Cdevelopment` so the Cursor entry matches the Claude entry byte for byte. Proof: `tests/cursor-mcp-contract.test.ts` "restricts the Supabase MCP to the docs and development feature groups" — asserts the URL and that it equals the `.mcp.json` URL. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
…oo (L40) Defect: the deny list spelled the write-capable Supabase MCP tools only as `mcp__supabase__*`, the `.mcp.json` server name. The claude.ai connector that `docs/claude-cloud.md` recommends for cloud sessions surfaces the same tools as `mcp__Supabase__*`, and whether the rule matcher is case-insensitive could not be verified offline — so `apply_migration` and friends may have sat in the default ask state on that route instead of being hard-denied. Trigger: a cloud session with the Supabase connector attached composing a write-capable tool call against the live clinical project. Fix: add a capitalised `mcp__Supabase__` twin for each of the eleven denied tools. Proof: `tests/claude-code-settings.test.ts` "denies every write-capable Supabase MCP tool under both the server and connector spellings". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
…twin (L62) Defect: `.cursor/skills/repo-auditor/SKILL.md` told the agent to "identify and safely remove" dead files, while the Claude twin (`.claude/agents/repo-auditor.md`) and AGENTS.md "Deleting code you believe is dead" make dead-code review triage-only with evidence and never a delete list. Trigger: a Cursor session running the repo-auditor skill on a file that is only reachable from a route, a package script, a migration, the worker, or a test. Fix: rewrite the dead-files bullet to candidates-only, add the "Safety before deletion" block from the Claude twin, and point at `docs/agents/dead-code-deletion.md` and `check:dead-code-candidate`. Proof: `tests/agent-scopes.test.ts` "cursor repo-auditor skill is triage-only" requires the three contract fragments in both twins and no "safely remove". (The L102 and L132 cases in the same describe block go green in the next two commits.) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
Defect: the "Database repository override" paragraph in `.cursor/skills/supabase/SKILL.md` applied only "when this skill runs in `C:\Dev\Apps\Database`", a workstation path the repository no longer records as its location, so the one prose guard keeping Cursor's Supabase MCP away from `execute_sql` read as not applying to the real checkout. Trigger: a Cursor session in any current checkout reading the override literally. Fix: both occurrences now say "this repository (any checkout of BigSimmo/Database)". The fragments `npm run check:skills` asserts are unchanged. Proof: `tests/agent-scopes.test.ts` "cursor supabase skill's repository override does not depend on a workstation path". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
Defect: `.cursorignore` began with the bytes EF BB BF, contrary to `.editorconfig` (`charset = utf-8`, BOM-less). Line 1 is a comment today, so no exclusion is affected, but a real pattern moved to line 1 would silently stop matching. Trigger: any future edit that puts an exclusion such as `.env*` on the first line. Fix: re-save the file without the BOM; no other byte changes. Proof: `tests/agent-scopes.test.ts` ".cursorignore has no UTF-8 byte-order mark". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1dd796e5-0a29-4e76-a478-36f23fb229af) |
Summary
Audit remediation package P9 — Agent tooling and permissions, from
docs/audit/full-repository-audit-2026-09-02.md(PR #2573). Findings fixed:M21,M35,L39,L40,L62,L102,L132.The one that matters is
M21..claude/settings.jsononmainallowsBash(git push:*)— every spelling of push, with any arguments. The deny list only catches a force flag written immediately aftergit pushorgit push origin, sogit push origin some-branch --forceis auto-approved today with no prompt. This replaces the blanket allow with three specific safe spellings and grows the deny list from 15 entries to 40, covering force, force-with-lease,-f,+refs, delete, mirror, andmain/mastertargets in every spelling the tool can produce.An independent verification pass worked the matcher against ten dangerous spellings; all ten are denied:
git push --forcegit push --force-with-leasegit push -fgit push origin +refs/heads/xgit push origin HEAD:maingit push origin maingit push --mirrorgit push --delete origin <branch>git push -u origin <feature>git push(plain)A residual gap remains, and it needs an owner decision — see "Known gap" below.
The rest:
M35widens the review-agent scope globs to the realsrc/lib/rag/**paths (the previous globs namedsrc/lib/rag*.ts, which no longer exists).L40adds the capitalisedmcp__Supabase__*deny twins for all eleven lowercase rows, so the connector cannot be reached under its other spelling.L39addsfeatures=docs,developmentto the Supabase MCP entries.L62makes the Cursor repo-auditor triage-only, matching its Claude twin.L102removes a hard-coded workstation path.L132strips a byte-order mark from.cursorignore.Known gap — owner decision required
Bash(git push -u origin claude/:*)is a raw string-prefix allow. That meansgit push -u origin claude/some-branch --forceand... --no-verifymatch the allow and reach no deny rule — verified by running the settings file's own matcher against both. This is precisely the shapeM21exists to prevent.It is not fixed here, for two reasons. First, this package still strictly improves on
main, where force-pushing any branch is auto-approved. Second, closing it means removing the convenience allow, after which every branch push by an assistant session stops and asks — a real daily cost that is the owner's call, not automation's.Recommended follow-up (not taken here): rather than removing the convenience rule, extend
scripts/guard-push.mjs, which inspects the actual push rather than the command text and therefore cannot be dodged by a spelling. Today it only blocks a force-push when a pull request on that branch has auto-merge armed.Findings in this package not fixed here, with reasons:
L41— pinning the Node andghtarball checksums requires fetching upstreamSHASUMS, a network call this work was not authorised to make.L61— the pull-request-handoff hook wording is pinned the opposite way by two committed tests; changing it needs an owner ruling on which wording is correct, and touches files this package does not own.RAG impact: none — assistant tooling configuration only; no retrieval, ranking, ordering, selection or product code changed.
Verification
npm run verify:pr-local—- completed: check:runtime, check:installed-lock-parity, format:changed, check:diff-integrity, sitemap:check, check:repo-awareness-snapshot, docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links, check:branch-review-ledger, check:outstanding-issues, check:ledger-write-discipline, check:github-actions, check:ci-scope, check:gitleaks-pinned, check:ci-triage, check:pr-policy, check:gate-manifest, check:skills, check:pr-mergeability, check:verification-plan, lint, typecheck, test, check:rag:fixtures, check:medication-interactions, check:medication-lexicon-report·- failed: (none)·- not reached: (none)·Test Files 1128 passed | 1 skipped (1129)·Tests 15118 passed | 2 expected fail | 3 skipped (15123)npm run check:diff-integrity—[diff-integrity] PASS — 3 changed test file(s), 20 -> 32 test case(s), against base 51ddfcd83.npm run check:skills—Database skill system valid: 35 canonical skills, 8 aliases, 67 repository SKILL.md files across 4 surfaces.L132verified byte-for-byte:origin/main's.cursorignorebeginsef bb bf; this branch's begins23 20 43. The mark is genuinely gone.M35verified by existence check:src/lib/rag*.tsgenuinely does not exist, and every file named in the newsrc/lib/rag/**scope does.Build skipped by the gate's own classifier:
Skipping build: no build-affecting source, config, package, or container changes detected.Verification not run:
npm run verify:ui— no UI surface changed; browser proof left to CI.Verification not run:
npm run verify:release— no release or handoff confidence is claimed.Verification not run: provider-backed gates — nothing here reaches OpenAI, Supabase, Railway or Sentry at run time.
Risk and rollout
main, not wider.Notes
.claude/hooks/is empty, asdocs/agents/claude-hook-scripts.mdrequires.origin/mainwas merged into this branch (not rebased) before gating.🤖 Generated with Claude Code
https://claude.ai/code/session_01DHSyfuC6mS98ystWFiitAR
Generated by Claude Code
Note
Low Risk
No runtime clinical/RAG behavior changes; risk is limited to assistant developer workflow (more push prompts) and a documented residual force-push prefix gap on
claude/*upstream pushes.Overview
Audit P9 (agent tooling) — no application or RAG product code changes; only Claude/Cursor config, review-agent docs, and tests.
Git push permissions (
M21) in.claude/settings.jsondrops the blanketBash(git push:*)allow and replaces it with three explicit safe patterns (git push,git push origin HEAD,git push -u origin claude/:…). The deny list grows to cover force/force-with-lease,+refspecs, remote delete/mirror,--no-verify, and main/master targets in additional spellings. Supabase MCP write tools are denied under bothmcp__supabase__*andmcp__Supabase__*connector spellings (L40).Review-agent scopes (
M35) retargetrag-retrieval-reviewerandclinical-governance-reviewerfrom obsoletesrc/lib/rag*.tsglobs tosrc/lib/rag/**after the RAG tree move. Newtests/agent-scopes.test.tsvalidates scope globs against the repo tree and Cursor “twin” contracts (repo-auditor triage-only, portable Supabase skill wording, BOM-free.cursorignore).Cursor Supabase MCP (
L39) addsfeatures=docs,developmentto match Claude/Codex and avoid default database/debugging tools against the live project. Repo-auditor and Supabase Cursor skills are aligned with Claude (L62,L102);.cursorignoreUTF-8 BOM removed (L132).Tests expand
claude-code-settings.test.tsfor push tightening and Supabase deny twins;cursor-mcp-contract.test.tsasserts the restricted Supabase URL. PR notes a residual gap:git push -u origin claude/… --forcecan still match the convenience allow prefix.Reviewed by Cursor Bugbot for commit 6ad1945. Configure here.