Skip to content

fix(action-surface): one permission list per action - #418

Merged
pengfei-threemoonslab merged 2 commits into
mainfrom
claude/hungry-villani-368127
Aug 26, 2026
Merged

fix(action-surface): one permission list per action#418
pengfei-threemoonslab merged 2 commits into
mainfrom
claude/hungry-villani-368127

Conversation

@pengfei-threemoonslab

@pengfei-threemoonslab pengfei-threemoonslab commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • One action, one permission list. A manifest row that listed scopes: and declared no authority: block at either site turned verify --base into Internal error (exit 4) on a legal manifest. The action's permissions were spelled twice and the two spellings disagreed: the action lens took the row's list when it had one and the source's auth scopes otherwise, while _assess_authority took the row's list only where a reviewed authority record existed. CapabilityFactV1._semantic_projection_is_consistent requires the two to project one list, so where they disagreed, rebuilding a capability fact from a serialized ActionFact raised.
  • This finishes what feat(evidence): authority follows credentials, not functions (#410 increment 3) #417 started. Declaring authority once per source closed the reviewed half of the divergence by normalizing both manifest sites into one record. What was left is every row that declares scopes with no reviewed authority at either site — 10 of the 28 shapes in the new sweep raised on current main.
  • The fix is one resolver, not a patched builder. core.semantic_assessment.resolve_action_scopes is the single derivation, read by the action lens (replacing _declared_scope_strings) and by the authority dimension. The alternative — have capability_fact_from_action_fact read the semantic authority instead of required_scopes — was rejected: it keeps two permission lists on one action (one published as required_scopes, one as the authority evidence) and leaves the invariant asserting a guarantee the engine no longer makes.
  • A row cannot quietly shrink a grant. Publishing the row's list on the authority dimension is also what would let scopes: [crm.read] erase a crm.write grant the source proves, with a structural status and nothing raised. The subset rule a reviewed authority is held to now reads the resolved list (_scopes_narrow_source), so a bare scopes: list is held to it too and reports conflicting_authority_evidence against actions[].scopes. Adding the dropped scope back closes it; broadening is still a broadening.
  • The parity assertion added with the source-level block (test_the_action_fact_and_the_assessment_publish_one_permission_list) explicitly excluded the bare-scopes shape because it did not hold. It now covers it, and its stale carve-out comment is gone.

Pre-existing on main (found while reviewing the #410 increment-3 branch; reproduces on main both before and after #417 landed).

Type

  • Check or risk-model change
  • Input adapter change
  • CLI or GitHub Action behavior
  • Report, schema, or SARIF output
  • Documentation only

Verification

CI is authoritative for python -m ruff check ., python -m compileall -q src tests, and python -m pytest.

Additional local checks run, all on the rebased tree (branch is rebased onto 6f55acc3, the merge of #417):

  • pytest -n auto -m "not perf" --ignore=tests/test_adapter_static_only.py — green; plus tests/test_adapter_static_only.py and tests/test_latency_budget.py -m perf as separate runs, both green. ruff check . clean.
  • The regression tests fail on pre-fix src/. git stash push -- src/ then re-run: all four tests in tests/test_action_scope_projection.py fail, including the real verify --base one at exit 4 with the exact CapabilityFactV1.authority.scopes must project semantic authority message.
  • The invariant is enforced on exactly one route — checks/mcp_permissions.py reaches capability_fact_from_action_fact only when a base diff reference exists — so the new coverage is a real verify --base run over the reported shape, not a unit assertion. It also asserts both reports' action surfaces are non-empty MCP rows, because an empty action surface would pass the test while proving nothing.
  • The 28-shape sweep was run against stashed src/ to measure the 10 divergences quoted above, and again after, to confirm all 28 agree. (Clear .git/agents-shipgate/base-scans/ before any such comparison — a cached base scan from a different build silently disables the diff and hides the crash.)

Release-readiness notes

  • No user-code import added to default scan paths
  • No network access added to default scan paths
  • New or changed check IDs are documented in docs/checks.md — none added or changed
  • Report/schema changes are additive or documented in STABILITY.md — no field added, removed, or retyped; docs/manifest-v0.1.json moves only by one regenerated description

Notes for the reviewer

  • Blast radius is smaller than "move authority.scopes" suggests. The concern with this direction was churn to authority_hash for every repo using bare scopes. It does not happen, because the shared rule is the one the lens already applied: every shape that resolves today resolves identically, and every list that moves belongs to a shape that was exit 4. No sample golden, fixture, or test value moved — no samples/*/shipgate.yaml row uses a bare scopes: list, and tests/test_evidence_backed_pass.py requires an authority block, which is why this survived.
  • One adopter-visible consequence beyond the crash. SHIP-AUTH-SCOPE-COVERAGE-MISSING reads the assessment's scopes, so a row declaring scopes: against a manifest whose permissions.scopes does not cover them now raises the review item it always should have. The divergence, not the check, was keeping it quiet. Called out in the CHANGELOG.

🤖 Generated with Claude Code

A manifest row that listed `scopes:` and declared no `authority:` block at
either site turned `verify --base` into `Internal error` (exit 4) on a legal
manifest. The action's permissions were spelled twice and the two spellings
disagreed: the action lens took the row's list when it had one and the
source's auth scopes otherwise, while `_assess_authority` took the row's list
only where a reviewed authority record existed.
`CapabilityFactV1._semantic_projection_is_consistent` requires the two to
project one list, so where they disagreed, rebuilding a capability fact from a
serialized `ActionFact` raised — and that rebuild happens on exactly one
route, the MCP capability comparison against a base scan. A plain `scan` never
reaches it, which is why no sample and no scan-level test saw this.

Declaring authority once per source (#410 increment 3) closed the reviewed
half of that divergence by normalizing both manifest sites into one record.
This closes the rest: `resolve_action_scopes` is the single derivation, read
by the action lens and the authority dimension alike, rather than teaching the
capability builder to paper over a disagreement it would then have to keep
tolerating. Ten of the twenty-eight shapes in the new sweep raised before it.
Because the shared rule is the one the lens already applied, no shape that
resolves today resolves differently — every list that moves belongs to a shape
that was exit 4.

Publishing the row's list on the authority dimension is also what would let
`scopes: [crm.read]` erase a `crm.write` grant the source proves, with a
`structural` status and nothing raised. The subset rule a reviewed authority
is held to now reads the *resolved* list, so a bare `scopes:` list is held to
it too and reports `conflicting_authority_evidence` against
`actions[].scopes`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pengfei-threemoonslab
pengfei-threemoonslab force-pushed the claude/hungry-villani-368127 branch from 141d3a0 to 7794749 Compare August 26, 2026 01:58
Review of the resolver caught it introducing a byte-identical copy of the
lens's `_normalize_strings` — a fresh second spelling of a rule, in the change
whose whole argument is that a rule with two spellings is a defect. Same body,
same output, and nothing forcing them to stay that way.

The primitive moves to `core/action_semantics.py`, the leaf module both
readers already import, under a name that fits both lists an
`action_surface.actions` row carries: `scopes` and `risk_tags` normalize
identically because comparing them is what decides whether a declaration
matches, broadens, or narrows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pengfei-threemoonslab
pengfei-threemoonslab merged commit aa6bd61 into main Aug 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant