sec (2/9): reject inline env values in user-defined scanners

[jesse-merhi]

What changes

A user-defined scanner's env: list must contain bare variable names
(API_TOKEN), never inline NAME=value pairs. An entry with an = is rejected
at config-resolution time, and — critically — the rejected value is never
echoed in the error.

Why it matters

env: exists so a scanner can be handed a secret that ClawScan reads from the
process environment and records only by presence (present/missing). If an
author writes API_TOKEN=sk-live-... inline, the secret would live in the
config file and could surface in logs/errors. Requiring bare names keeps secret
values out of config entirely, matching the repo rule "record env presence,
never values."

Before / after (runnable)

env.yml:

version: 1
profiles:
  demo:
    scanners:
      - id: foo
        command: "scan {{target}}"
        env:
          - "API_TOKEN=sk-live-secret"

	Behavior
Before	inline NAME=value accepted; secret value carried in config
After	rejected, and the error omits the value:

$ clawscan ./skill --config env.yml --profile demo --sandbox off --json
User-defined scanner foo in profile demo has an invalid env entry "API_TOKEN";
declare bare variable names and set values in the environment, not inline

The reject message names API_TOKEN but does not contain sk-live-secret.
A bare env: [API_TOKEN] is still accepted.

Verify

go test ./internal/profiles/ -run 'UserDefinedScannerEnv' -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, 7:22 AM ET / 11:22 UTC.

ClawSweeper review

What this changes

The PR adds profile-resolution validation that rejects inline values and malformed names in user-defined scanner env: declarations, with tests for allowed and rejected inputs.

Merge readiness

⛔ Blocked by patch quality or review findings - 6 items remain

Keep open. The validation path is narrowly useful, but it can still echo a secret when an invalid env: item has no = (for example, a pasted token), so the stated secret-safe diagnostic guarantee is incomplete.

Priority: P1
Reviewed head: 302c0ad09782c4e668d632e9fdbf4fedff980a4a

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch has a focused security goal and tests, but a remaining secret-disclosure path blocks merge readiness.
Proof confidence	🌊 off-meta tidepool	Not applicable: The PR author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the supplied focused test command remains useful supplemental validation.
Patch quality	🦪 silver shellfish (2/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The PR author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the supplied focused test command remains useful supplemental validation.
Evidence reviewed	4 items	Repository secret-handling policy: The repository policy requires scanner credentials to be environment variables, artifacts to record only present or missing, and sandbox environment passthrough to be an allowlist of names rather than values. Raw invalid declaration can reach the error: The new helper assigns an invalid entry directly to name and returns it unchanged when there is no = or space. Thus an invalid bare token such as sk-live-secret is included in the configuration error, despite the PR's no-secret-echo goal. Current test coverage misses the no-equals secret case: The added tests verify that API_TOKEN=sk-live-secret is redacted, but they do not cover a raw invalid secret-shaped entry without an equals sign, which follows the leaking return path.
Findings	1 actionable finding	[P1] Avoid echoing raw invalid environment entries
Security	Needs attention	Malformed env values can be emitted in errors: For invalid declarations without = or spaces, the new helper returns the raw string to the configuration error. A pasted token in env: can therefore be written to terminal output or logs.

How this fits together

Profile configuration creates user-defined scanner adapters before ClawScan executes them. The declared environment-variable names are then passed to the sandbox and represented in scan artifacts only as presence state, so resolver diagnostics must not expose configured secret material.

flowchart LR
  A[Profile YAML] --> B[Profile resolver]
  B --> C[Scanner env validation]
  C --> D[Scanner registry]
  D --> E[Sandboxed scanner run]
  E --> F[Secret-safe run artifact]

Loading

Before merge

• Avoid echoing raw invalid environment entries (P1) - name begins as the complete invalid declaration and is returned unchanged when it has neither = nor a space. A profile entry such as sk-live-secret therefore reaches the resolver error verbatim, contradicting the intended secret-safe diagnostic boundary. Return only a demonstrably safe identifier for the inline-assignment case; otherwise use a redacted placeholder or entry index.
• Resolve security concern: Malformed env values can be emitted in errors - For invalid declarations without = or spaces, the new helper returns the raw string to the configuration error. A pasted token in env: can therefore be written to terminal output or logs.
• Resolve merge risk (P1) - Merging as written can place a malformed secret value without = into configuration errors or logs.
• Resolve merge risk (P1) - Existing user-defined scanner profiles that used inline NAME=value declarations will stop resolving after upgrade; that security migration should remain explicit and diagnosable without exposing values.
• Complete next step (P2) - A narrow, source-local redaction repair and regression test can resolve the blocking security finding without changing the intended bare-name policy.
• Improve patch quality - Make invalid-entry diagnostics fully value-free and add regression tests for raw secret-shaped malformed entries.

Findings

• [P1] Avoid echoing raw invalid environment entries — internal/profiles/resolver.go:980
• [medium] Malformed env values can be emitted in errors — internal/profiles/resolver.go:980

Agent review details

Security

Needs attention: The patch improves the credential boundary overall, but its malformed-input diagnostic can still disclose raw secret material.

Review metrics

Metric	Value	Why it matters
Patch surface	2 files affected; 100 additions, 0 deletions	The security hardening is concentrated in resolver validation and its unit coverage, so a complete redaction fix should remain small and reviewable.

Merge-risk options

Maintainer options:

1. Redact every invalid declaration (recommended)
   Change the error path so malformed entries never contribute raw configuration text, then add regression coverage for secret-shaped values with and without an equals sign.
2. Accept the configuration migration
   Retain rejection of existing inline-value declarations, but ensure the release or PR context makes the move to process environment variables clear without logging the old values.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Update invalid environment-entry diagnostics to avoid returning raw invalid input, add focused tests for no-equals and malformed secret-shaped entries, and retain the bare-name migration behavior.

Technical review

Best possible solution:

Keep the bare-name requirement, but make every invalid-entry diagnostic value-free: report a safe identifier only for a validated pre-= name and otherwise use a generic entry position or redacted placeholder; add regression cases for raw secret-shaped and malformed declarations.

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

Yes, by source inspection. Configure a custom scanner with an invalid env: entry containing a secret-shaped value but no =, then resolve the profile; the helper returns that raw entry for the error message.

Is this the best way to solve the issue?

No. Rejecting inline values is the right boundary, but returning the complete malformed entry when no delimiter is present leaves an unredacted secret path; a generic or position-based diagnostic is safer.

Full review comments:

• [P1] Avoid echoing raw invalid environment entries — internal/profiles/resolver.go:980
name begins as the complete invalid declaration and is returned unchanged when it has neither = nor a space. A profile entry such as sk-live-secret therefore reaches the resolver error verbatim, contradicting the intended secret-safe diagnostic boundary. Return only a demonstrably safe identifier for the inline-assignment case; otherwise use a redacted placeholder or entry index.
  Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1ade9ae9be17.

Labels

Label changes:

• add P1: The proposed security validation can itself disclose configured secrets in an error path for malformed entries.
• add merge-risk: 🚨 compatibility: Profiles that currently use inline NAME=value declarations will fail configuration resolution after upgrade.
• add merge-risk: 🚨 security-boundary: The changed validation and diagnostic path handles scanner credential declarations and can expose raw secret text.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the supplied focused test command remains useful supplemental validation.

Label justifications:

• P1: The proposed security validation can itself disclose configured secrets in an error path for malformed entries.
• merge-risk: 🚨 compatibility: Profiles that currently use inline NAME=value declarations will fail configuration resolution after upgrade.
• merge-risk: 🚨 security-boundary: The changed validation and diagnostic path handles scanner credential declarations and can expose raw secret text.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The PR author is a repository member, so the external-contributor real-behavior-proof gate does not apply; the supplied focused test command remains useful supplemental validation.

Evidence

Security concerns:

• [medium] Malformed env values can be emitted in errors — internal/profiles/resolver.go:980
  For invalid declarations without = or spaces, the new helper returns the raw string to the configuration error. A pasted token in env: can therefore be written to terminal output or logs.
  Confidence: 0.97

Acceptance criteria:

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

What I checked:

• Repository secret-handling policy: The repository policy requires scanner credentials to be environment variables, artifacts to record only present or missing, and sandbox environment passthrough to be an allowlist of names rather than values. (AGENTS.md:276, 1ade9ae9be17)
• Raw invalid declaration can reach the error: The new helper assigns an invalid entry directly to name and returns it unchanged when there is no = or space. Thus an invalid bare token such as sk-live-secret is included in the configuration error, despite the PR's no-secret-echo goal. (internal/profiles/resolver.go:980, 302c0ad09782)
• Current test coverage misses the no-equals secret case: The added tests verify that API_TOKEN=sk-live-secret is redacted, but they do not cover a raw invalid secret-shaped entry without an equals sign, which follows the leaking return path. (internal/profiles/resolver_test.go:1118, 302c0ad09782)
• User-defined-scanner provenance: The merged BYOS foundation introduced profile-defined scanner commands and declared environment names; this PR is a focused hardening follow-up on that same configuration boundary. (internal/profiles/resolver.go:317, 656e558b4ba5)

Likely related people:

• jesse-merhi: Authored the merged BYOS implementation that established profile-defined scanner environment declarations and the current focused hardening PR. (role: user-defined scanner feature owner; confidence: high; commits: 656e558b4ba5, 302c0ad09782; files: internal/profiles/resolver.go, internal/profiles/resolver_test.go)

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.