Skip to content

feat: fix 7 read-only gaps found by audit log analysis - #19

Merged
grams merged 1 commit into
mainfrom
audit-log-quick-fixes
Aug 31, 2026
Merged

grams merged 1 commit into
mainfrom
audit-log-quick-fixes

Conversation

@grams

@grams grams commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes found by replaying blocked audit-log entries through --audit: real commands that were being denied despite doing nothing but read.

  • gh api graphql: checked by GraphQL operation type (mutation/subscription anywhere in the document, or an operationName field) instead of the REST write-method heuristic, which doesn't apply since the query document is itself passed as a -f/-F field. Endpoint detection uses a dedicated flags-with-value map so a flag's own value (e.g. graphql passed to -X/--preview) can't be mistaken for the endpoint and skip the write-method check.
  • gcloud run <resource> <verb> (including behind alpha/beta) no longer swept into the generic run write-verb blacklist; its own verb is checked at its expected position. auth application-default restricted to the read-only token printers.
  • git reset: non-destructive forms (no mode flag, --soft, --mixed) allowed via a flag allowlist rather than blacklisting --hard/--merge/--keep — git accepts unambiguous abbreviations (--har for --hard) that a blacklist misses.
  • npx: strips a trailing @version before matching the allowlist (prettier@3), left untouched for alias/path/URL references (prettier@npm:x) which aren't a version.
  • systemctl show/list-timers and comm added (read-only).
  • uv run --with-requirements <file> -- <cmd> left as-is: the real subcommand sits after --, which every allowlisted tool treats as a hard stop on purpose (tested elsewhere against smuggling a value past a positional check).

Test plan

  • go build ./... && go vet ./... && go test ./...
  • gofmt -l . clean
  • Local review: pr-review-toolkit:code-reviewer (3 rounds — found and fixed real bypasses: GraphQL mutation-detection evasion via leading comment/fragment/comma and operationName, gh api endpoint-detection shadowing via -X/--preview values, npm alias/file/git-URL references treated as a version, Cloud Run write verbs reachable via an unrecognized flag shifting token positions, glued-form -f/-F REST fields) + Kilroy (clean, one comment tightened)
  • Empirically verified against real git/gh/gcloud/npx binaries, not just unit tests
  • Scope check: stopped chasing further pflag short-flag-clustering bypasses (gh api -if title=pwned) — this tool targets prompt reduction for commands an LLM writes innocently, not hardening against a deliberately adversarial one (already the file's stated posture: "best-effort filter, not a security boundary")

@grams grams added the transverse Transverse plateforme (plusieurs BUs ou outillage plateforme) label Aug 31, 2026
@grams grams self-assigned this Aug 31, 2026
gh api graphql: check the GraphQL operation type (deny mutation/
subscription anywhere in the document, and any operationName field)
instead of the REST write-method heuristic, which doesn't apply since
the query document is itself passed as a -f/-F field. Endpoint
detection now uses a dedicated flags-with-value map for `gh api`, so a
flag's own value (e.g. "graphql" passed to -X/--preview) can no longer
be mistaken for the endpoint positional and skip the write-method
check entirely. hasFieldOrInputFlag also recognizes -f/-F forms with
the value glued to the flag.

gcloud: Cloud Run (`run <resource> <verb>`, optionally behind
alpha/beta) is no longer swept into the generic "run" write-verb
blacklist; its own verb is checked at its expected position instead of
scanning every token, so an unrecognized flag's separate-argument
value can't be mistaken for it. auth application-default restricted to
the read-only token printers.

git reset: allow the forms that don't touch the working tree (no mode
flag, --soft, --mixed) via an allowlist of flags rather than a
blacklist of destructive ones — git accepts unambiguous abbreviations
of long flags (`--har` for `--hard`), which a blacklist can't reliably
catch.

npx: strip a trailing "@Version" before matching against the
allowlist, so "prettier@3" works like "prettier". Left untouched when
what follows "@" is an alias/path/URL reference (contains ":" or "/"),
which isn't a version.

Also allow systemctl show/list-timers and comm (read-only).

uv run --with-requirements <file> -- <cmd> is left as-is: the real
subcommand sits after "--", which every other allowlisted tool also
treats as a hard stop on purpose.
@grams
grams force-pushed the audit-log-quick-fixes branch from 09577f0 to 94323ff Compare August 31, 2026 20:51
@grams
grams merged commit 36ea4ed into main Aug 31, 2026
2 checks passed
@grams
grams deleted the audit-log-quick-fixes branch August 31, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

transverse Transverse plateforme (plusieurs BUs ou outillage plateforme)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant