sec (3/9): require an active {{target}} in user-defined scanners

[jesse-merhi]

What changes

A user-defined scanner command must contain at least one active
{{target}} placeholder — one that is not inside shell quotes and not after a
# comment. A command with no active placeholder is rejected at
config-resolution time.

Why it matters

The whole point of a scanner command is that ClawScan substitutes the target
path into it. If {{target}} is missing (or only appears quoted/commented), the
scanner runs against nothing — the run looks like it scanned the skill but the
tool never saw it, producing a false "clean". Failing loudly at config time
prevents a silently no-op scanner.

Before / after (runnable)

notarget.yml:

version: 1
profiles:
  demo:
    scanners:
      - id: foo
        command: "scanner --scan"

	Behavior
Before	accepted; scanner runs without ever receiving the target
After	run aborts:

$ clawscan ./skill --config notarget.yml --profile demo --sandbox off --json
User-defined scanner foo in profile demo must include an active {{target}}
placeholder outside shell quotes and comments so the scanner receives the target

A placeholder that is only inside quotes, or only after a #, is likewise
rejected; a normal scan {{target}} is accepted.

Verify

go test ./internal/profiles/ -run 'TargetPlaceholder' -count=1

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[clawsweeper]

Codex review: needs changes before merge. Reviewed July 25, 2026, 10:37 PM ET / July 26, 2026, 02:37 UTC.

ClawSweeper review

What this changes

The branch validates user-defined scanner commands at profile resolution time and rejects those without an active {{target}} placeholder outside shell quotes and comments.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep this PR open. It addresses a real scan-integrity problem, but the new comment detector does not match shell comment boundaries and can reject valid multiline scanner commands before they run.

Priority: P2
Reviewed head: 17a2615c42f0f54503ee03f6c61f38829bcdbbf4

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The security hardening is focused and useful, but a concrete shell-parsing compatibility defect should be corrected before merge.
Proof confidence	🌊 off-meta tidepool	Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the PR body also supplies a focused test command for the changed validation path.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the PR body also supplies a focused test command for the changed validation path.
Evidence reviewed	4 items	New parser treats all following text as a comment: The branch adds persistent comment state while scanning the full command string; it is not reset on a newline, so a valid later-line {{target}} is not counted as active. Focused coverage misses multiline shell-comment behavior: The added tests cover no placeholder and a placeholder only in a comment, but do not cover a comment on one line followed by an active placeholder on a later line or an inline # within a shell word. Feature provenance: User-defined scanner support entered current main through the merged BYOS feature, making its author the strongest available owner signal for this validation path.
Findings	1 actionable finding	[P2] Parse shell comments at their actual boundaries
Security	None	None.

How this fits together

Profile configuration turns user-defined scanner declarations into scanner adapters before a scan begins. This validation sits between YAML profile resolution and command execution, where a target path is passed to the scanner and its raw JSON output becomes run evidence.

flowchart LR
  A[Profile YAML] --> B[Custom scanner definition]
  B --> C[Target placeholder validation]
  C --> D[Scanner registry]
  D --> E[Sandboxed scanner command]
  E --> F[Raw JSON scan evidence]

Loading

Before merge

• Parse shell comments at their actual boundaries (P2) - The new comment flag never resets at a newline and activates for every unquoted #. Consequently, a valid block command such as echo # note\nscanner {{target}} is rejected even though the shell executes the second line with the target; arg=prefix#{{target}} is likewise not a shell comment. Reset state per line and only start comments where # begins a shell word.
• Resolve merge risk (P1) - Merging this parser as written can break existing valid multiline custom-scanner commands: shell comments end at a newline, and # inside a shell word is not a comment delimiter.

Findings

• [P2] Parse shell comments at their actual boundaries — internal/profiles/resolver.go:1074

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch surface	2 files affected; 59 added, 6 removed	The change is narrowly scoped to profile validation and its focused tests, so the shell-lexing edge case is mechanically repairable.

Merge-risk options

Maintainer options:

1. Align comment parsing with shell syntax (recommended)
   Reset comment state at line boundaries and recognize # only when it begins a shell word, with regression tests for a later-line target and an inline hash.

Technical review

Best possible solution:

Make placeholder classification follow shell comment boundaries—begin comments only where the shell would and end them at a newline—then add focused regression coverage for multiline commands and inline hashes.

Do we have a high-confidence way to reproduce the issue?

No direct runtime reproduction was executed in this read-only review, but source inspection gives a high-confidence path: a multiline command with a comment on one line and scanner {{target}} on the next is treated as having no active target.

Is this the best way to solve the issue?

No. Requiring an active target is the right safeguard, but the implementation should use shell-accurate comment boundaries rather than treating every later character after an unquoted # as commented.

Full review comments:

• [P2] Parse shell comments at their actual boundaries — internal/profiles/resolver.go:1074
  The new comment flag never resets at a newline and activates for every unquoted #. Consequently, a valid block command such as echo # note\nscanner {{target}} is rejected even though the shell executes the second line with the target; arg=prefix#{{target}} is likewise not a shell comment. Reset state per line and only start comments where # begins a shell word.
  Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5cfa90b129f4.

Labels

Label changes:

• add P2: A valid custom-scanner configuration can be rejected before scanning, but the defect has a bounded compatibility impact and a focused repair path.
• add merge-risk: 🚨 compatibility: The new configuration-time validation can reject existing multiline commands that the shell would execute with an active target argument.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the PR body also supplies a focused test command for the changed validation path.

Label justifications:

• P2: A valid custom-scanner configuration can be rejected before scanning, but the defect has a bounded compatibility impact and a focused repair path.
• merge-risk: 🚨 compatibility: The new configuration-time validation can reject existing multiline commands that the shell would execute with an active target argument.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the PR body also supplies a focused test command for the changed validation path.

Evidence

Acceptance criteria:

• [P1] go test -count=1 ./internal/profiles/.
• [P1] go vet ./...

What I checked:

• New parser treats all following text as a comment: The branch adds persistent comment state while scanning the full command string; it is not reset on a newline, so a valid later-line {{target}} is not counted as active. (internal/profiles/resolver.go:1074, 17a2615c42f0)
• Focused coverage misses multiline shell-comment behavior: The added tests cover no placeholder and a placeholder only in a comment, but do not cover a comment on one line followed by an active placeholder on a later line or an inline # within a shell word. (internal/profiles/resolver_test.go:1218, 17a2615c42f0)
• Feature provenance: User-defined scanner support entered current main through the merged BYOS feature, making its author the strongest available owner signal for this validation path. (internal/profiles/resolver.go:1003, 656e558b4ba5)
• Current-main comparison: The current main SHA is 5cfa90b129f4b54b45b7aed02989078cab27c9b7; this validation change exists only on the reviewed branch, so the central hardening is not already implemented on main. (internal/profiles/resolver.go:1003, 5cfa90b129f4)

Likely related people:

• jesse-merhi: Authored merged user-defined-scanner support and the follow-up validation change in this same profile-resolution path. (role: feature introducer and recent area contributor; confidence: high; commits: 656e558b4ba5, 17a2615c42f0; files: internal/profiles/resolver.go, internal/profiles/resolver_test.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Correct comment-boundary handling and add regression tests for multiline commands and inline hashes.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

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