feat(safety): enforce runtime read-only mode#866
Conversation
|
Codex review: found issues before merge. Reviewed June 22, 2026, 1:48 PM ET / 17:48 UTC. Summary Reproducibility: not applicable. this is a feature PR rather than a reported current-main bug. The PR body reports targeted tests plus live Gmail and Calendar CLI validation, and I verified the source-level change shape without running tests in the read-only checkout. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the opt-in runtime read-only boundary after maintainer acceptance of the POST allowlist, clean CI, and moving release-note text out of Do we have a high-confidence way to reproduce the issue? Not applicable; this is a feature PR rather than a reported current-main bug. The PR body reports targeted tests plus live Gmail and Calendar CLI validation, and I verified the source-level change shape without running tests in the read-only checkout. Is this the best way to solve the issue? Yes, with maintainer policy review; enforcing at the authenticated transport layer is a maintainable place to block API mutations before dispatch while preserving OAuth refresh. The remaining cleanup is to remove the release-owned changelog edit and finish normal CI/review gates. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0122f09d84e5. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
11578b2 to
557b1d2
Compare
|
Landed as Verification:
Landing fixes: host-and-path-scoped POST query allowlist, method-override rejection, guided-auth and Discovery preflight enforcement, shared Discovery query classification, and sticky readonly setup guidance. |
Summary
Adds a global runtime
--readonlysafety backstop (GOG_READONLY=1) that rejects mutating Google and Zoom API requests before network dispatch.This complements, rather than replaces:
--gmail-no-send;--dry-runplanning.gog auth add --readonlyremains CLI-compatible. The flag now has one consistent global meaning: request read-only OAuth scopes during authorization and enforce read-only behavior on later commands.Enforcement model
The default Google API client stack now wraps its authenticated transport when read-only mode is active:
--readonly.Policy violations use stable usage exit code 2 and preserve
errors.Is(err, googleapi.ErrReadOnly)for embedding callers.Schema and docs
gog schema --jsonnow reportsautomation.safety.readonly. The global flag appears throughout the generated command reference; 699 command pages and the docs site were regenerated and checked after rebasing on guided auth and Discovery API support.Validation
make ciinternal/googleapiandinternal/cmdtestsNotes.Createis blockedclawmac.local:--readonly: exit 0--readonly: exit 2,request blocked by --readonly, no stdout, no network dispatch--readonly: exit 0Safety notes
The live mutation test used a dummy token and was rejected locally. No email was sent, no Calendar data changed, no Zoom meeting changed, and no Pub/Sub message was consumed.