Skip to content

fix(approvals): drop quoted free-text operands from stored approval patterns (#1406) - #1815

Merged
Aaronontheweb merged 1 commit into
devfrom
fix/1406-quoted-freetext-approval-pattern
Aug 8, 2026
Merged

fix(approvals): drop quoted free-text operands from stored approval patterns (#1406)#1815
Aaronontheweb merged 1 commit into
devfrom
fix/1406-quoted-freetext-approval-pattern

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Problem

A single-line quoted operand flowed verbatim into the stored shell approval
pattern. Every unique value became a new pattern. The gate re-prompted for each
one. Two examples:

  • git commit -m "fix the bug" kept fix the bug in the pattern, so every
    commit message re-prompted.
  • freshdesk ticket reply --message "Single line body" kept the body.

The existing termination rules did not fire. IsCallSpecificValueToken needs a
digit in the token. The #1402 rule needs an embedded line break.

The fix (predicate 1)

The change adds one rule to ReconstructClauseText. A quote-wrapped arg whose
decoded text holds internal whitespace is call-specific free text. The rule
drops it from the normalized pattern. It runs beside the digit (#1331) and
multi-line (#1402) rules. The composition is digit OR multi-line OR whitespace-quoted.

Predicate 1 is the conservative choice. A single-word quoted arg has no
internal whitespace, so the pattern keeps it. git commit -m "fix" still keeps
fix, and a quoted and an unquoted single token normalize the same way. Only a
multi-word quoted value operand drops.

Security reasoning

  • Pattern-only. The change shapes the stored pattern and the approve-retry
    key only. It does not change the live authorization decision. That decision
    re-parses each command and scopes every path arg through the zone gate.
    FormatForDisplay still shows the full command to the operator, because a
    single-line command has no line break and takes the verbatim fast path.
  • Path args keep their scope. The drop applies to value tokens only. The
    parser marks a real path IsPath, and the rule exempts it. A quoted path
    with a space (cat "my file.txt") keeps its directory scope, which
    ExtractCandidates resolves from the same parsed arg. A quoted search
    pattern (grep "foo bar" ./notes.txt) drops, and the trailing path still
    scopes the candidate.
  • Wrappers are unaffected. The analyzer expands bash -c and sh -c
    command strings into inner clauses before this walk, so the rule never hides
    an inner command from the gate. Prefix wrappers such as env and timeout
    keep their existing termination behavior.

Tests

New cases in ShellApprovalMatcherTests:

  • git commit -m "fix the bug" normalizes to git commit -m.
  • freshdesk ticket reply --message "Single line body" drops the body.
  • git commit -m "fix" keeps fix (single-word quoted).
  • find . -name "*.cs" keeps the glob (no internal whitespace).
  • cat "my file.txt" keeps its directory scope in ExtractCandidates.
  • grep "foo bar" ./notes.txt keeps the path scope and ignores the free text.
  • FormatForDisplay shows the full command for a single-line message.

No existing test needed a normalization update. git commit -m fix and
bash -c "git push --force" stay unchanged, because neither carries a
multi-word quoted value operand.

Validation

  • dotnet test src/Netclaw.Security.Tests — 678 passed, 0 failed, 0 skipped.
  • Downstream approval and compaction tests in Netclaw.Actors.Tests — 437
    passed, 0 failed.
  • dotnet slopwatch analyze — 0 issues.
  • Add-FileHeaders.ps1 -Verify — all files have headers.

Spec follow-up

The openspec/specs/tool-approval-gates/spec.md "Shell command pattern
matching" requirement defines call-specific classification as one
morphological rule (digit-bearing). This change adds the quoted-free-text
rule. Reconcile the spec through /opsx-sync. Do not hand-edit it.

…atterns (#1406)

A single-line quoted operand flowed verbatim into the stored shell approval
pattern. Every unique value became a new pattern. The gate re-prompted for
each one. `git commit -m "fix the bug"` and `freshdesk ticket reply --message
"Single line body"` are two examples.

The existing termination rules did not fire. The digit rule needs a digit in
the token. The #1402 rule needs an embedded line break.

This change adds a third rule to ReconstructClauseText. A quote-wrapped arg
whose decoded text holds internal whitespace is call-specific free text. The
rule drops it from the normalized pattern. It runs beside the digit (#1331)
and multi-line (#1402) rules.

The rule is conservative. A single-word quoted arg has no internal whitespace,
so the pattern keeps it. `git commit -m "fix"` still keeps `fix`, and a quoted
and an unquoted single token normalize the same way. Only a multi-word quoted
value operand drops.

Security scope:

- The change shapes the stored pattern and the approve-retry key only. It does
  not change the live authorization decision. That decision re-parses each
  command and scopes every path arg through the zone gate.
- The drop applies to value tokens only. A path arg is exempt. A quoted path
  with a space (`cat "my file.txt"`) keeps its directory scope, which
  ExtractCandidates resolves from the same parsed arg.
- Wrapper verbs are unaffected. The analyzer expands `bash -c` and `sh -c`
  command strings into inner clauses before this walk. Prefix wrappers such as
  `env` and `timeout` keep their existing termination behavior.

@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

@Aaronontheweb
Aaronontheweb merged commit 4836e88 into dev Aug 8, 2026
21 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/1406-quoted-freetext-approval-pattern branch August 8, 2026 01:35
@Aaronontheweb Aaronontheweb added shell Issues related to the shell tool, since it has the largest security perimeter. bug Something isn't working UX/DX UI / UX / DX friction issue or user-facing annoyances. labels Aug 8, 2026
Aaronontheweb added a commit that referenced this pull request Aug 8, 2026
…-gates (#1406) (#1816)

Reconcile the spec with the behavior that PR #1815 merged. Add the
quoted-free-text termination rule to the "Shell command pattern matching"
requirement. The rule sits next to the digit-bearing rule and the
multi-line rule. A quoted argument with internal whitespace is
call-specific free text and stops pattern extraction.

Add four scenarios that pin the shipped constraints:
- a multi-word quoted operand drops from the pattern
- multi-word quoted operands generalize across values
- a single-word quoted argument stays in the pattern
- a quoted path keeps directory scoping

Archive the completed change under openspec/changes/archive/.
@Aaronontheweb Aaronontheweb mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working shell Issues related to the shell tool, since it has the largest security perimeter. UX/DX UI / UX / DX friction issue or user-facing annoyances.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant