Skip to content

Normalize version/value args out of shell approval verb chains#1388

Merged
Aaronontheweb merged 3 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/shell-approval-version-arg-normalization
Jun 10, 2026
Merged

Normalize version/value args out of shell approval verb chains#1388
Aaronontheweb merged 3 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/shell-approval-version-arg-normalization

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Summary

Fixes an approval-gate inconsistency where semantically-identical shell commands were gated differently based on the leading character of a version argument.

ShellSyntaxTree's greedy verb walk (SPEC §6.1) folds a lowercase-leading version like v0.4.2 into the verb chain (git tag v0.4.2), but a digit-leading 0.4.2 stops the walk and lands in Args (verb stays git tag). Because ShellApprovalMatcher compared the full verb chain for exact equality against persisted grants, git tag 0.4.2 matched a standing git tag grant and ran instantly while git tag v0.4.2 missed it and re-prompted.

This extends the issue #1331 call-specific-value rule (bare integers) to version-shaped tokens, applied on both the gate path (ExtractCandidatesViaBashParser) and the persisted/display path (ReconstructClauseText) so they normalize identically. git tag v0.4.2 and git tag 0.4.2 now both normalize to git tag.

Verification

  • Build clean; 578 Netclaw.Security.Tests + 311 actor approval/dispatch tests pass.
  • 12 new ShellApprovalMatcherTests cases (parity, persist-path, IsApproved scenarios, conservatism boundary).
  • File headers present; Slopwatch clean for changed files (5 new POSIX-only tests baselined per repo convention).

Draft — open review items

A high-effort review surfaced follow-ups to triage before marking ready:

  1. Spec not updatedopenspec/specs/tool-approval-gates/spec.md ("Shell command pattern matching") still lists only flag/path/URL/bare-integer as stop conditions; needs a version-token delta.
  2. Predicate over-matches non-versionsIsVersionShapedToken matches IPv4/dates (8.8.8.8, 2024.06.01); for network commands the destination operand gets normalized away.
  3. break drops trailing flagsgit tag 0.4.2 --sign persists git tag (order-dependent); consider continue vs break.
  4. Cleanup: dedupe the trim+join across the two call sites; char-class inconsistency (IsDigit vs IsAsciiDigit).
  5. POSIX-only fix; Windows legacy path unchanged (disclosed scope boundary).

ShellSyntaxTree's greedy verb walk (SPEC §6.1) folds a lowercase-leading
version token such as `v0.4.2` into the verb chain (`git tag v0.4.2`),
while a digit-leading `0.4.2` stops the walk and lands in Args (verb
stays `git tag`). Because ShellApprovalMatcher compared the full
`clause.Verb.Joined` for exact equality against persisted grants, the
same intent produced two different verbs: `git tag 0.4.2` matched a
standing `git tag` grant and ran instantly, while `git tag v0.4.2`
missed it and re-prompted. SPEC §6.1.1 warns that `Clause.Verb` is a
convenience hint, not a security contract, and that consumers must do
their own verb normalization.

Extend the issue netclaw-dev#1331 call-specific-value rule (bare integers) to also
cover version-shaped tokens, and apply it on both verb-chain paths so
they normalize identically:

- ExtractCandidatesViaBashParser (the gate) and ReconstructClauseText
  (the persisted/display pattern) run the verb tokens through
  TrimTrailingValueTokens, dropping trailing bare-integer and
  version-shaped tokens while always retaining the command word.
- IsVersionShapedToken requires a dot after the optional v/V+digit, so
  v0.4.2 / 1.0.0-beta.3 / 1.0+build collapse but v2, branch names, and
  hex SHAs are preserved.
- Verb equality matching is unchanged, so no other grant is broadened.

`git tag v0.4.2` and `git tag 0.4.2` now both normalize to `git tag`,
and a freshly-granted version generalizes across versions instead of
pinning to the one approved.

The fix targets the POSIX BashParser path; the Windows legacy
ShellTokenizer path takes a different route and is unchanged (tests are
POSIX-gated accordingly).

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumb

Comment thread src/Netclaw.Security/IToolApprovalMatcher.cs Outdated
Replace the version-shape predicate (and the bare-integer special case
it composed with) with a single morphological rule: a token is a
call-specific value iff it is not a flag, not path-shaped, and contains
a digit. This deletes IsVersionShapedToken and IsBareIntegerToken in
favor of one IsCallSpecificValueToken.

The shape taxonomy approach accretes special cases (integers, dotted
versions, next SHAs, IPs, dates...) while still mis-drawing boundaries:
`git checkout v2` was preserved but `git checkout v2.0` stripped, and
alpha-leading SHAs (`git show aa211dc`) never normalized, so a
standing `git show` grant could not cover the dominant SHA use-case.
Under the digit rule versions, SHAs, range refs, IDs, and digit-bearing
ref names all normalize uniformly.

Boundaries that keep the rule safe:
- Trailing-only trim with a one-token floor: mid-chain tokens (`aws s3
  ls`) and command heads (`python3`) are never touched.
- Flags are exempt (`-3`, `--max-count=10` carry intent, not values).
- Path-shaped tokens are exempt so digit-bearing paths still reach
  directory scoping and display patterns.
- All-alpha operands (branch names, package names) are intentionally
  not classified: no shape rule distinguishes them from subcommands,
  and mis-stripping a subcommand silently widens a grant.
Documents the implemented value-token normalization in the "Shell
command pattern matching" requirement: the bare-integer stop condition
(issue netclaw-dev#1331) generalizes to one morphological rule (non-flag, non-path
token containing a digit), and trailing value tokens folded into the
verb chain by greedy extraction are trimmed identically on the gate and
persisted-pattern paths.

The prior "Non-bare numeric tokens are preserved" scenario no longer
holds under the generalized rule and is replaced by "Digit-bearing
operand terminates the pattern" (docker run --name test123 --port=8080
persists `docker run --name`), now pinned by a test. Scenarios added
for version-prefix parity, folded SHA trimming, and the trailing-only
boundary.

OpenSpec change: shell-approval-value-token-normalization (proposal,
design, delta spec, tasks; synced to main spec, archival pending PR
merge).
@Aaronontheweb Aaronontheweb marked this pull request as ready for review June 10, 2026 21:58

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

[InlineData("git log v0.4.1..dev", "git log")] // range ref is a value
[InlineData("git push origin main", "git push origin main")] // all-alpha operands are unclassifiable by shape -> preserved
[InlineData("aws s3 ls", "aws s3 ls")] // mid-chain digit token is not trailing -> untouched
public void ExtractCandidateVerbs_trims_digit_bearing_tokens_trailing_only(string command, string expected)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit 208c2ab into netclaw-dev:dev Jun 10, 2026
23 of 24 checks passed
@Aaronontheweb Aaronontheweb deleted the fix/shell-approval-version-arg-normalization branch June 10, 2026 21:59
Aaronontheweb added a commit that referenced this pull request Jun 11, 2026
…ization (#1389)

All tasks complete (3.3 verify + 3.4 archive). PR #1388 merged at
208c2ab. Delta spec was synced to openspec/specs/tool-approval-gates/
in the PR branch; no further sync needed.
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