Fix stack 05: monotonic permission ladder, honor READ_ONLY, Antigravity posture#29
Merged
ebarti merged 4 commits intoJul 2, 2026
Merged
Conversation
ebarti
force-pushed
the
review-fixes/04-finish-reason
branch
from
f47fda4 to
f088d0f
Compare
ebarti
force-pushed
the
review-fixes/05-permissions
branch
2 times, most recently
from
a471d02 to
1af8343
Compare
…posture Claude (security-critical): - Permission ladder was non-monotonic: CAUTIOUS -> 'acceptEdits' (auto-approves edits and in-cwd deletes) was strictly looser than DEFAULT -> 'default'. A caller picking CAUTIOUS 'to be careful' got unattended file mutation. Claude has no distinct cautious tier, so CAUTIOUS now maps to 'default' like DEFAULT. - permissions.filesystem was ignored entirely, so READ_ONLY ran read-write on Claude while Codex/Antigravity honored it. READ_ONLY now forces 'plan' mode. - The effective vendor permission_mode is surfaced in AgentResult.metadata['permission_mode']. Antigravity: - Explicit vertex=True is no longer silently overridden by an ambient GEMINI_API_KEY/GOOGLE_API_KEY; explicit config takes precedence. - READ_ONLY filesystem with an explicit allowed_tools list no longer bypasses the read-only restriction; a write tool is rejected (visible) instead of silently widening access. - An explicitly allow-listed start_subagent now enables subagents regardless of mode (was silently inert outside PERMISSIVE). - Validated allow/deny tools are returned as enum members, matching the type the default-tool paths hand CapabilitiesConfig. Updates the capability-matrix permission table to match. Refs REVIEW.md §1.4, §1.5, §2b.
…r SDK drift If the installed SDK no longer exposes a read-only toolset helper, the READ_ONLY + allowed_tools check silently skipped verification — the same fail-open-under-drift class fixed for permission kwargs. Refuse instead. Also documents the auth precedence (constructor api_key > explicit vertex=True > ambient env key) in providers.md.
…_ONLY/STRICT The READ_ONLY hard constraint was enforced only on the allow-list path; the deny-list path built disabled_tools (= enable everything else), so READ_ONLY + disallowed_tools left every unnamed write tool enabled and STRICT + deny-list bypassed the documented read-only posture. Both constraints combine instead: enabled_tools = read-only toolset minus the denied tools, sharing the fail-closed read-only resolution with the allow-list guard. Documents the rule and the allow-listed-subagent rule.
…TIOUS deny-lists within baseline
Two posture gaps in _capability_policy:
- An explicit allow-list under STRICT enabled any named write tool
(run_command included) even though STRICT is a read-only posture
everywhere else in the adapter. The read-only guard now applies under
READ_ONLY or STRICT, matching the deny-list branch and _default_tools.
- A deny-list under DEFAULT/CAUTIOUS took the SDK's disabled_tools route
("enable everything else"), so denying one unrelated tool re-enabled
run_command and every other destructive tool past the nondestructive
baseline. Deny-lists are now subtractive for every mode: PERMISSIVE
keeps disabled_tools (baseline is all tools, so it is exact); everyone
else gets baseline-minus-denied as an allow-list, failing closed via a
typed error if the SDK stops exposing the nondestructive toolset.
Docs updated to describe the per-mode deny-list mapping.
ebarti
force-pushed
the
review-fixes/05-permissions
branch
from
e222a2a to
eccd48c
Compare
ebarti
force-pushed
the
review-fixes/04-finish-reason
branch
from
b7e38ae to
cb1c5be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked review-fix PR 5/N (base:
review-fixes/04-finish-reason).Claude (security-critical, REVIEW.md §1.4/§1.5):
CAUTIOUS→acceptEdits(auto-approves edits and in-cwd deletes) was looser thanDEFAULT→default.CAUTIOUSnow maps todefault.permissions.filesystemwas ignored, soREAD_ONLYran read-write on Claude.READ_ONLYnow forcesplanmode.AgentResult.metadata["permission_mode"].Antigravity (§2b, §4):
vertex=Trueno longer silently overridden by an ambientGEMINI_API_KEY.READ_ONLY+ explicitallowed_toolsno longer bypasses read-only; write tools are rejected (visible).start_subagentenables subagents regardless of mode.Capability-matrix permission table updated to match. Full suite 162 passed / 12 skipped; ruff + mypy strict clean.
Re-review amendments.
_reject_non_read_only_toolsnow fails closed when the installed SDK exposes no read-only toolset to verify against (vendor drift previously made the READ_ONLY +allowed_toolscheck silently skip verification — the same fail-open class fixed in the drift-filtering PR). providers.md now documents the auth precedence: constructorapi_key> explicitvertex=True> ambientGEMINI_API_KEY/GOOGLE_API_KEY.Second re-review round (independent Antigravity reviewer). Found and fixed a MUST-FIX this PR's own hardening had only half-applied: the READ_ONLY hard constraint guarded the allow-list path but not the deny-list path.
disabled_toolsmeans "enable everything else", soREAD_ONLY + disallowed_tools=("run_command",)left every unnamed write tool enabled, and STRICT + deny-list bypassed the documented read-only posture. Both constraints are simultaneously expressible and now combine:enabled_tools = read-only toolset minus the denied tools, sharing the fail-closed read-only resolution with the allow-list guard. Tests pin READ_ONLY and STRICT deny-list conversion; providers.md documents the rule and the allow-listed-subagent rule.Round 2 (re-review follow-up)
allowed_toolsnaming a write tool underSTRICTis now rejected (the read-only guard applies underREAD_ONLYorSTRICT, matching the deny-list branch and the default toolsets).DEFAULT/CAUTIOUSdeny-lists no longer re-enablerun_commandand other destructive tools past the nondestructive baseline: deny-lists are subtractive in every mode, and onlyPERMISSIVE(baseline = all tools) still uses the SDK'sdisabled_toolsroute. Fails closed with a typed error if the SDK stops exposing the nondestructive toolset.providers.md.