Skip to content

feat(safety): enforce runtime read-only mode#866

Merged
steipete merged 7 commits into
mainfrom
codex/runtime-readonly
Jun 22, 2026
Merged

feat(safety): enforce runtime read-only mode#866
steipete merged 7 commits into
mainfrom
codex/runtime-readonly

Conversation

@steipete

@steipete steipete commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a global runtime --readonly safety backstop (GOG_READONLY=1) that rejects mutating Google and Zoom API requests before network dispatch.

This complements, rather than replaces:

  • least-privilege OAuth scopes;
  • command allow/deny rules;
  • baked safety profiles;
  • --gmail-no-send;
  • command-specific --dry-run planning.

gog auth add --readonly remains 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:

  • GET, HEAD, and OPTIONS pass through;
  • mutating POST, PUT, PATCH, and DELETE requests return a policy error before the underlying transport runs;
  • known Google query APIs that use POST remain available through a narrow HTTPS host-and-path allowlist, including Calendar free/busy, Analytics reports, Search Console queries/inspection, Drive Activity queries, Sheets data-filter reads, and Photos search;
  • OAuth token refresh remains functional because the policy wraps the API request transport, not the internal token exchange;
  • account, direct-access-token, and service-account clients share the same enforcement;
  • the separate Keep service-account path now uses the wrapped HTTP client;
  • Zoom meeting create/delete and Pub/Sub-acknowledging Gmail watch pull are explicitly blocked;
  • guided auth setup cannot create projects or enable APIs under read-only mode, while dry-run planning and read-only OAuth login remain available;
  • Discovery write methods fail before confirmation/auth, while allowlisted Discovery query POSTs use the same classifier as the transport;
  • MCP child processes inherit --readonly.

Policy violations use stable usage exit code 2 and preserve errors.Is(err, googleapi.ErrReadOnly) for embedding callers.

Schema and docs

gog schema --json now reports automation.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 ci
  • targeted internal/googleapi and internal/cmd tests
  • direct transport tests prove reads and allowlisted query POSTs dispatch while Gmail send POSTs never reach the base transport
  • Keep service-account regression test proves Notes.Create is blocked
  • Gmail watch pull regression test proves Pub/Sub acknowledgment is blocked
  • MCP propagation and schema snapshot tests
  • live E2E on clawmac.local:
    • real Gmail labels read under --readonly: exit 0
    • Gmail send with a dummy access token under --readonly: exit 2, request blocked by --readonly, no stdout, no network dispatch
    • Calendar free/busy query under --readonly: exit 0
  • autoreview: final run clean after fixing the Keep service-account bypass, host-scoped query allowlist, guided-auth and Discovery integrations, shared query classification, and readonly next-step guidance

Safety 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.

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 22, 2026, 1:48 PM ET / 17:48 UTC.

Summary
The PR adds global --readonly / GOG_READONLY=1 runtime enforcement across authenticated Google transports, selected Zoom/Gmail/Discovery paths, MCP/schema propagation, tests, and regenerated command docs.

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.

  • Changed files: 721 files changed. Most review volume is generated command documentation rather than handwritten runtime logic.
  • Generated command pages: 699 command pages regenerated. Reviewers should treat the command-reference churn as generated output and focus source review on the smaller implementation surface.
  • Handwritten Go surface: 19 internal Go files changed or added. The runtime behavior is concentrated in command, Google API client, and focused test files.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Remove the CHANGELOG.md entry from the PR diff.
  • Wait for the remaining CI jobs to complete successfully.
  • Have a maintainer explicitly review the allowed POST query policy before treating this as a safety boundary.

Risk before merge

  • [P1] The read-only boundary relies on HTTP method plus host/path allowlisting for POST query APIs; maintainers should explicitly accept that policy before relying on it as an agent containment boundary.
  • [P1] CI was still reported as unstable with test and Windows jobs in progress at inspection time, so merge should wait for the required checks to complete.
  • [P1] The PR still edits release-owned CHANGELOG.md; that line should be removed from the branch and kept as PR-body or landing/release context.

Maintainer options:

  1. Audit allowed POST paths
    Confirm each allowed Google POST host/path is read-only for reachable gog services and that no mutating endpoint can share the suffix match.
  2. Land as an opt-in boundary
    Maintainers may accept the current classifier with the added tests and PR-body live validation if the allowlist policy is considered narrow enough.
  3. Pause for service-by-service policy
    If the allowlist needs broader ownership review, keep the PR open until the permanent read-only policy is settled.

Next step before merge

  • [P2] This collaborator feature PR should stay in maintainer review for allowlist policy acceptance, CI completion, and the release-owned changelog cleanup rather than being closed or routed to automated repair.

Security
Cleared: No concrete supply-chain, dependency, or secret-handling concern was found; the security-sensitive part is the intended new runtime enforcement boundary and its allowlist policy.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:9
Review details

Best 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 CHANGELOG.md.

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:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:9
    OpenClaw release-note policy keeps CHANGELOG.md release-owned for normal PR review. Please keep this context in the PR body or commit message and let the landing/release flow update the changelog.
    Confidence: 0.86

Overall correctness: patch is correct
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0122f09d84e5.

Label changes

Label justifications:

  • P2: This is a meaningful opt-in safety feature with broad runtime surface, but it is not an urgent regression or emergency.
  • merge-risk: 🚨 security-boundary: The PR creates a new read-only enforcement boundary whose correctness depends on transport coverage and POST allowlist policy.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • feature: ✨ showcase: ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. A network-dispatch-level read-only backstop is unusually valuable for agents operating over broad Google account scopes.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR; the body still reports live CLI validation for readonly Gmail and Calendar behavior.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its PR-link review mode says to inspect via gh pr view / gh pr diff and not switch branches or change code. (AGENTS.md:25, 0122f09d84e5)
  • PR metadata and status: The PR is open, no longer draft, authored by collaborator steipete, changes 721 files with 1140 additions and 40 deletions, and GitHub reported it mergeable but unstable with several CI jobs still in progress at inspection time. (557b1d28e78b)
  • Current main does not already implement runtime enforcement: Current main has auth-time auth add --readonly scope selection, but no GOG_READONLY, ErrReadOnly, or runtime read-only transport implementation, so the central feature is not obsolete on main. (internal/cmd/auth_add.go:31, 0122f09d84e5)
  • PR adds the read-only classifier and transport: The PR branch adds ErrReadOnly, context state, a RoundTripper guard, and a host/path allowlist for POST query APIs. (internal/googleapi/read_only.go:11, 557b1d28e78b)
  • PR wraps authenticated Google transports: Account, ADC, direct-token, and service-account client options are routed through readOnlyTransportFromContext, which is the intended pre-dispatch enforcement point. (internal/googleapi/client.go:151, 557b1d28e78b)
  • PR still edits release-owned changelog: The PR branch adds a release note under the unreleased section; ClawSweeper review policy treats CHANGELOG.md as release-owned for normal PRs. (CHANGELOG.md:9, 557b1d28e78b)

Likely related people:

  • steipete: Current main history shows repeated work on root flags, auth, Google API client construction, automation discovery, and baked safety profiles; the PR author also has prior merged ownership in these paths. (role: recent area contributor and feature owner; confidence: high; commits: f26af3adba6d, 02291d290e87, 0122f09d84e5; files: internal/cmd/root.go, internal/cmd/auth_add.go, internal/googleapi/client.go)
  • drewburchfield: Merged safety-profile hardening work is adjacent to this PR's safety-boundary behavior and policy review surface. (role: adjacent safety contributor; confidence: medium; commits: 46900109e029; files: internal/cmd/safety_profile.go, internal/safetyprofile/hash.go, internal/safetyprofile/parse.go)
  • salmonumbrella: Earlier readonly Gmail/Drive scope handling is the auth-time behavior that this PR keeps compatible while adding runtime enforcement. (role: prior readonly-auth contributor; confidence: medium; commits: 25040fe7fe23; files: internal/cmd/auth_add.go, internal/googleauth/service.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. feature: ✨ showcase ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jun 22, 2026
@steipete steipete force-pushed the codex/runtime-readonly branch from 11578b2 to 557b1d2 Compare June 22, 2026 17:39
@steipete steipete marked this pull request as ready for review June 22, 2026 17:40
@steipete steipete merged commit 4cac149 into main Jun 22, 2026
9 checks passed
@steipete steipete deleted the codex/runtime-readonly branch June 22, 2026 18:00
@steipete

Copy link
Copy Markdown
Collaborator Author

Landed as 4cac149d75a7835d3b54a68c512ef66e2639996e.

Verification:

  • make ci — passed locally after final integration and lint fixes; 699 generated command pages and docs coverage passed.
  • Focused internal/googleapi and internal/cmd tests — passed for transport enforcement, query POST classification, guided auth, Discovery calls, Keep, Gmail watch, MCP, schema, and exit codes.
  • Codex autoreview against current main — final run clean, no accepted/actionable findings.
  • GitHub CI — all 9 checks passed, including Linux, macOS CGO, Windows, worker, and Docker checks.
  • Existing live proof: Gmail read and Calendar free/busy succeeded under readonly; a dummy-token Gmail send was rejected locally with exit 2 and no dispatch.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: ✨ showcase ClawSweeper spotlight: unusually compelling feature idea for maintainer attention. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant