fix(opencode): redact short credential values from scan output

[yashrajp22]

Configured credentials shorter than four characters were omitted from OpenCode output redaction. Redact those values as whole tokens so common placeholders do not corrupt words, risk scores or flags. JSON output is scrubbed inside strings while preserving numeric and boolean metadata.

Merge overlapping configured-value and key-shaped matches on the original text so partial or adjacent matches cannot expose another credential. Assignment detection consumes each name once to avoid repeatedly scanning long uppercase output. Redaction runs before truncation on model-visible success/error paths. The separate report file written by --output is outside this redactor.

Validation: all 56 OpenCode helper tests pass, including short placeholders, adjacent full keys and assignments, JSON metadata, punctuation-bearing and overlapping credentials, truncation and error paths. The 128 KiB repeated-uppercase regression completed in about 10 ms in the isolated test container.

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for tightening the OpenCode tool's credential redaction and testing it on every success and failure path through truncation!

Value and readiness: The narrow goal is met: a configured 1-3 character value is now replaced on every model-visible path, and redaction still runs before truncation. Removing the length floor without bounding how short values are matched causes two regressions, and neither is tested:

• A one-character placeholder now defeats the existing sk- key patterns, so most of a real key leaks.
• A one-character placeholder also corrupts the JSON report the agent reads.

This is not ready yet.

Material findings

1. [Blocker] .opencode/tools/skillspector_scan_lib.ts:47 (with :58-61): Short configured values now split key-shaped tokens before the sk-ant-, sk- and NAME=value patterns get to them, because the configured-value pass runs first.

   • Example: OPENAI_API_KEY=x (a common placeholder for local OpenAI-compatible servers), and a scanned skill whose flagged line contains api_key="sk-proj-Q7wErTyxUiOp9AsDfGhJkL". Finding code_snippet text reaches stdout verbatim, because the CLI's redact_text only scrubs URL credentials.
   • On main the whole key becomes [REDACTED]. With this PR the x pass yields sk-proj-Q7wErTy[REDACTED]UiOp…. \bsk-…\b then matches only up to the bracket, so the model sees [REDACTED][REDACTED]UiOp9AsDfGhJkL.
   • Any given letter or digit appears in about half of random 48-character keys and in most longer ones, so this is not a corner case.

   Expected: apply the prefix and NAME=value patterns before the configured-value matcher, or in the same pass. Add a regression with a short configured value plus an unrelated sk-…/sk-ant-… key in the output. Reordering also closes the same, rarer split for configured values of 4+ characters.

2. [Blocker] .opencode/tools/skillspector_scan_lib.ts:47: Literal substring replacement of 1-2 character values rewrites ordinary report text.

   • With OPENAI_API_KEY=1, "risk_score": 12 becomes "risk_score": [REDACTED]2 and "start_line": 1 becomes "start_line": [REDACTED]. The default --format json output is no longer valid JSON.
   • With =e, true becomes tru[REDACTED] and "severity" becomes "s[REDACTED]v[REDACTED]rity".
   • The agent gets an unparseable or unreadable verdict, with nothing telling it why.
   • The new tests pad with o/e and check "before 7 after", so they never show what happens to ordinary text.

   Expected: either keep a minimum length for global substring replacement, or redact values shorter than 4 characters only as standalone tokens (not adjacent to [A-Za-z0-9_-]). Add a test that a representative JSON report stays parseable and readable when a one-character placeholder such as x or e is configured.

PIC tradeoffs: No provider in CREDENTIAL_ENV_NAMES issues keys shorter than 4 characters. A 1-3 character value is therefore almost certainly a placeholder, and if it were a real secret it would be trivially guessable. The PIC should decide whether such values are redacted at all (current main skips them) or only as whole tokens. Either way, the two blockers above must hold.

Verification and gaps:

• Output paths: I traced every model-visible path. These are success stdout and stderr (formatSuccess), partial streams and the message fallback (formatExecError), and the cancel, timeout and usage branches. All go through redact() before truncate(), so the PR's coverage claim holds.
• Not covered (pre-existing, all lengths): the --output report file is written by the CLI and is not redacted. The literal matcher also misses encoded forms of a value: JSON-escaped, percent-encoded, base64, or line-wrapped.
• Both regressions above were reproduced with a standalone Python model of the same pass order, using my own code, not the PR's. The regex semantics match JS for these inputs.
• Node tests were not executed, per policy. CI is action_required with no runs.
• The four later commits are bot merges of main. The PR's own diff is identical to the original commit, and the merged changes (#727, #731) touch unrelated files. git merge-tree against current main is clean.
• #747 edits the same file in different hunks and merges cleanly with this PR, so either can land first.

---

Decision: Changes Requested (reviewed head 95c11bb3a3b5845fe77494a8b354c33a1bdf38d8)

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for fixing both problems at the root and backing the fix with thorough overlap and JSON tests!

Value and readiness: Both blockers from the last review are fixed. Each has a regression test that fails on the previous head.

• Key splitting: configured values and key-shaped matches are now both found on the original text and merged into one redaction, so a short placeholder can no longer split an sk- key.
• Short values: values shorter than 4 characters match only as standalone tokens.
• JSON reports: when stdout is a JSON document, only string tokens are rewritten, so scores, line numbers and booleans stay intact.

The rework also closes three leaks that exist on main today (see Verification). Ready for final maintainer/PIC review. The finding and the PIC tradeoff below are optional.

Previous findings (review at 95c11bb. Since then: author commits 4ff755c5, 9604577a and 263b76e7, plus bot merges of main.)

• [Blocker] Short-value pass leaks sk- key tails: Resolved.
  • redact() now collects two span streams over the original text: configured values (.opencode/tools/skillspector_scan_lib.ts:53-63) and one combined key/assignment pattern (:64-68). It then merges overlapping or adjacent spans into a single [REDACTED] (:86-111).
  • A value shorter than 4 characters must not touch [A-Za-z0-9_-] (:57). sk- keys use the same character set, so a short value can never match inside a key.
  • The last review's input was OPENAI_API_KEY=x with api_key="sk-proj-Q7wErTyxUiOp9AsDfGhJkL" in a JSON code_snippet. It now becomes api_key="[REDACTED]".
  • The test at tests/opencode/skillspector_scan_lib.test.ts:100-107 pins this with x and Q7wE. Both cases fail at 95c11bb, and Q7wE also fails on main.
• [Blocker] A one-character placeholder corrupts the JSON report: Resolved.
  • If stdout parses as a JSON object or array, only string tokens are decoded, scrubbed and re-encoded (:113-118). Everything else, including indentation, is left as is.
  • With =1, =e or =x, an indent=2 report like the CLI's stays valid JSON. risk_score: 12, start_line: 1 and true are unchanged, and the severity key keeps its spelling.
  • The test at :125-140 pins this with 1, e and x. At 95c11bb, the 1 and e cases produce invalid JSON.

Material findings

1. [Non-blocking] .opencode/tools/skillspector_scan_lib.ts:64-79: key-shaped matches are now found on the original text. As a result, a configured value glued directly to a following key no longer creates the word boundary that main relied on.
   • Example: with NVIDIA_INFERENCE_KEY=nvapi-ABCD1234efgh, the text nvapi-ABCD1234efghsk-abcdef123456 becomes [REDACTED]sk-abcdef123456. Likewise, nvapi-ABCD1234efghOPENAI_API_KEY=second-secret keeps second-secret visible.
   • On main: both are fully redacted, because the inserted [REDACTED] marker gives \bsk- and \b[A-Z] a boundary to match against.
   • Why optional: this needs a configured value of 4 or more characters that ends in a letter, digit or _ and is followed by the next secret with no separator. The configured value itself is always hidden, and I found no realistic SkillSpector output with this layout.
   • Suggested fix: also run keyPattern over a copy of the text in which configured-value spans are replaced by a non-word character of the same length, and merge those spans as well. That keeps the code linear. A test such as redact("nvapi-ABCD1234efghsk-abcdef123456", { NVIDIA_INFERENCE_KEY: "nvapi-ABCD1234efgh" }) === "[REDACTED]" would pin it.

PIC tradeoffs: Standalone-token matching still rewrites standalone numbers in non-JSON formats when the placeholder is a digit.

• Example: with OPENAI_API_KEY=1, the Markdown row | Score | 1/100 | becomes | Score | [REDACTED]/100 |, and SKILL.md:1 becomes SKILL.md:[REDACTED].
• Not affected: JSON and SARIF output (the tool defaults to JSON), and letter placeholders such as x.
• Option: this is the "whole tokens" option from the last review. If the PIC would rather skip values shorter than 4 characters entirely, as main does, the overlap and JSON fixes still stand on their own.

Verification and gaps:

• Model: I wrote my own Python model of the new redact(), not the PR's code. Python re in ASCII mode matches JS for these patterns on ASCII input. The model reproduces every expectation in the new tests.

• Exact coverage: I compared the merged output against a brute-force oracle on 100,000 random inputs, including self-overlapping values such as abab. The oracle marks every occurrence of each configured value using its full surrounding context, plus the regex's own key matches. All 100,000 outputs matched.

• Comparison with main: I tracked every original character through main's sequential passes. I used 200,000 random inputs, plus 200,000 inputs built around realistic configured values. Apart from finding 1, the PR never shows a character that main hides, except in two narrow cases that expose no key material:

  • a trailing - after an sk-... token that contains an inner sk-ant- (\b stops before trailing dashes, while main's first pass took them);
  • configured values that themselves contain API_KEY, TOKEN, = or :.

  Inserting a separator after each configured value removes every finding-1 difference.

• Leaks on main that this closes:

  • a configured value of 4 or more characters inside an unrelated key. For example, Q7wE leaves sk-proj-[REDACTED]rTyxUiOp9AsDfGhJkL on main.
  • overlapping configured values. For example, abcd plus cdef leaves [REDACTED]ef on main.
  • JSON output where an assignment or sk- key starts a snippet line after an escaped \n. On main, \b fails after the n of \n, so "import os\nAWS_SECRET_ACCESS_KEY=..." and "...\nsk-abcdef..." pass through unredacted. The PR scrubs the decoded string and redacts both.

• JSON validity: 30,000 random reports (compact and indent=2) with configured values of 1-9 characters all stay parseable, with numbers, floats, booleans and null unchanged. For --format json without --output, the CLI prints only the report to stdout (cli.py:371). Progress output and transitive-scan warnings go to stderr. The JSON branch therefore applies in the tool's default flow.

• Display boundary, quotes and URLs:

  • I placed sk- keys, configured values and short values across the 12,000-character display limit at a range of offsets. No partial prefix stayed visible, because redaction still runs before truncation on every path.
  • Short values are redacted inside quotes, brackets and URLs (https://x:x@host/x?k=x) and after : or =. Inside x86, xyz, x-y and x_y they are left alone.

• Linear time: my model's run time doubles when the input doubles (ratios 1.96-2.07 between 200,000 and 400,000 characters). The inputs were:

  • long word runs;
  • sk- followed by a run of dashes;
  • NAME= followed by spaces;
  • repeated API_KE near-misses;
  • the value aaaa over a run of a, which hits the lookahead at every position;
  • a repeated standalone x;
  • JSON with one very large string, and JSON with many small strings.

• Merged main:

  • The #747 timeout code is intact (:9, :228, :578, :613, :622).
  • The gemini provider added in #496 uses Vertex AI credentials (GOOGLE_CLOUD_PROJECT), not an API-key variable, so CREDENTIAL_ENV_NAMES needs no new entry.
  • git merge-tree against main is clean, and the patch-id is identical at 263b76e7 and at the head. No other open PR touches these files.

• CI: on 263b76e7, "OpenCode TypeScript Tests", lint and DCO passed, and test-unit was still running. The head 81ab513 is a bot merge with CI action_required.

• Not run: contributor tests, per policy.

• Review thread: you replied to my inline thread and resolved it. Both parts of it are fixed.

---

Decision: Approved (reviewed head 81ab5137cad55510619dce4cb8e5ed0b4247426c)

[yashrajp22]

Thanks for spotting this! Credentials directly after a configured value are now caught on the original text, so overlapping values cannot split another key. I also made assignment detection scan each name once, which keeps long uppercase output from slowing it down. All 56 Node tests pass, including the adjacent-key, JSON, overlap, and runtime cases.