From ff5feaeb7f113d22ef58e91beae96a7813d23862 Mon Sep 17 00:00:00 2001 From: Morgan Lunt Date: Mon, 8 Jun 2026 14:43:53 -0700 Subject: [PATCH] code-modernization: never write discovered credential values into findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Legacy systems often contain live credentials, and assessment/findings files get committed and shared. Previously the security-auditor agent reported hardcoded secrets verbatim into ASSESSMENT.md and SECURITY_FINDINGS.md. - security-auditor: mandatory secret-handling rules — mask all credential values (file:line + 2-4 char preview), redact secrets from echoed tool output, recommend rotation for anything that looks live - assess/harden: gitignore-verified SECRETS.local.md quarantine file for the per-credential inventory; findings files get masked entries and a pointer only - new --show-secrets flag opts into raw values in the quarantine file (and only there) - README: document the behavior and advise users of earlier versions to check for already-committed findings and rotate --- plugins/code-modernization/README.md | 4 +++ .../agents/security-auditor.md | 25 +++++++++++++- .../commands/modernize-assess.md | 23 +++++++++++-- .../commands/modernize-harden.md | 34 +++++++++++++++++-- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/plugins/code-modernization/README.md b/plugins/code-modernization/README.md index 8678d696..9c1903b7 100644 --- a/plugins/code-modernization/README.md +++ b/plugins/code-modernization/README.md @@ -24,6 +24,10 @@ mkdir -p legacy && ln -s /path/to/your/legacy/codebase legacy/billing `/modernize-assess` works best with [`scc`](https://github.com/boyter/scc) (LOC + complexity + COCOMO) or [`cloc`](https://github.com/AlDanial/cloc), and falls back to `find`/`wc` if neither is installed. Portfolio mode also benefits from [`lizard`](https://github.com/terryyin/lizard) (cyclomatic complexity). The commands degrade gracefully without them, but the metrics will be coarser. +## Secret handling + +Legacy systems routinely contain live credentials, and assessment artifacts get committed and shared. `/modernize-assess` and `/modernize-harden` therefore **never write discovered credential values into findings files** — findings cite `file:line` with a masked preview (`AKIA****`). When credentials are found, a per-credential inventory (type, location, blast radius, rotation recommendation) is written to `analysis//SECRETS.local.md`, which the commands gitignore before writing. Pass `--show-secrets` to include raw values in that quarantine file (and only there). If you ran an earlier version of this plugin on a real system, check whether `analysis/` artifacts containing credentials were committed or shared, and rotate anything that was. + ## Commands The commands are designed to be run in order, but each produces a standalone artifact so you can stop, review, and resume. diff --git a/plugins/code-modernization/agents/security-auditor.md b/plugins/code-modernization/agents/security-auditor.md index f1b291db..4f34a340 100644 --- a/plugins/code-modernization/agents/security-auditor.md +++ b/plugins/code-modernization/agents/security-auditor.md @@ -39,7 +39,30 @@ terminal/screen items don't apply to a SPA. Work through what's relevant: Use available SAST where it helps (npm audit, pip-audit, grep for known-bad patterns) but **read the code** — tools miss logic flaws. Show tool output -verbatim, then add your manual findings. +verbatim — except secret values, which you redact (see below) — then add +your manual findings. + +## Secret handling (mandatory) + +Legacy codebases routinely contain live production credentials, and your +findings get pasted into decks, tickets, and committed markdown. Copying a +secret into a report multiplies the exposure you were hired to find. + +When you discover a hardcoded credential, API key, token, connection +string, or private key: + +- **Never write the secret's value into any output** — no finding table, + no report, no quoted code excerpt, no echoed tool output. Mask it to the + first 2–4 identifying characters plus `****` (`AKIA****`, + `postgres://app_user:****@db-prod…`). If a scanner prints a secret, + redact it before including the excerpt. +- Cite `file:line`. The source file is the canonical location — anyone who + legitimately needs the value can open it there. +- State what the credential appears to grant access to (database, queue, + cloud account, third-party API) and whether it looks like a production + or test credential. +- Recommend rotation for anything that looks live — exposure in source + means it is already compromised, independent of any modernization plan. ## Reporting standard diff --git a/plugins/code-modernization/commands/modernize-assess.md b/plugins/code-modernization/commands/modernize-assess.md index 188c2d43..fba8b8cc 100644 --- a/plugins/code-modernization/commands/modernize-assess.md +++ b/plugins/code-modernization/commands/modernize-assess.md @@ -1,6 +1,6 @@ --- description: Full discovery & portfolio analysis of a legacy system — inventory, complexity, debt, effort estimation -argument-hint: | --portfolio +argument-hint: [--show-secrets] | --portfolio --- **Mode select.** If `$ARGUMENTS` starts with `--portfolio`, run **Portfolio @@ -113,7 +113,9 @@ Spawn three subagents **in parallel**: 3. **security-auditor** — "Scan legacy/$1 for security vulnerabilities: injection, auth weaknesses, hardcoded secrets, vulnerable dependencies, missing input validation. Return findings in CWE-tagged table form with - file:line evidence and severity." + file:line evidence and severity. Mask every discovered credential value + per your secret-handling rules — file:line plus a 2–4 character masked + preview, never the value itself." Wait for all three. Synthesize their findings. @@ -141,6 +143,23 @@ need explained. ## Step 6 — Write the assessment +**Secrets quarantine first.** The assessment gets shared and committed — +discovered credential values must never appear in it. If the +security-auditor found any hardcoded credentials: + +1. Ensure `analysis/.gitignore` exists and contains the line + `SECRETS.local.md` (create or append as needed). If the project is a + git repo, verify with `git check-ignore -q analysis/$1/SECRETS.local.md` + before writing any findings. +2. Write `analysis/$1/SECRETS.local.md`: one row per credential — masked + preview, `file:line`, credential type, what it grants access to, + production/test guess, rotation recommendation. Only if the user passed + `--show-secrets`, add the raw value column here — this file only, never + ASSESSMENT.md. +3. In ASSESSMENT.md, the Security Findings section lists credential + findings with masked values only, plus a one-line pointer: + "Credential inventory in SECRETS.local.md (gitignored; not for sharing)." + Create `analysis/$1/ASSESSMENT.md` with these sections: - **Executive Summary** (3-4 sentences: what it is, how big, how risky, headline recommendation) - **System Inventory** (the scc table + tech fingerprint) diff --git a/plugins/code-modernization/commands/modernize-harden.md b/plugins/code-modernization/commands/modernize-harden.md index b8f7a72c..ce3a784b 100644 --- a/plugins/code-modernization/commands/modernize-harden.md +++ b/plugins/code-modernization/commands/modernize-harden.md @@ -1,6 +1,6 @@ --- description: Security vulnerability scan with a reviewable remediation patch — OWASP, CWE, CVE, secrets, injection -argument-hint: +argument-hint: [--show-secrets] --- Run a **security hardening pass** on `legacy/$1`: find vulnerabilities, rank @@ -9,6 +9,25 @@ them, and produce a reviewable patch for the critical ones. This command never edits `legacy/` — it writes findings and a proposed patch to `analysis/$1/`. The user reviews and applies (or not). +## Step 0 — Secrets quarantine setup + +Findings files get shared, committed, and pasted into decks — discovered +credential values must never land in them. Before any scanning: + +1. Ensure `analysis/.gitignore` exists and contains the line + `SECRETS.local.md`. Create the file or append the line if missing. +2. If the project is a git repo, verify with + `git check-ignore -q analysis/$1/SECRETS.local.md` — if that exits + non-zero, fix the ignore rule before proceeding. Do not write any + findings until this check passes (skip the check only if there is no + git repo). + +All secret values in every artifact this command produces are **masked** +(`AKIA****`, `password=****`) and cited by `file:line`. The one exception: +if the user passed `--show-secrets`, raw values may appear in +`analysis/$1/SECRETS.local.md` (gitignored above) and nowhere else — +never in SECURITY_FINDINGS.md or the patch commentary. + ## Scan Spawn the **security-auditor** subagent: @@ -20,7 +39,9 @@ hardcoded secrets, vulnerable dependency versions, missing input validation, path traversal. For each finding return: CWE ID, severity (Critical/High/Med/Low), file:line, one-sentence exploit scenario, and recommended fix. Run any available SAST tooling (npm audit, pip-audit, -OWASP dependency-check) and include its raw output." +OWASP dependency-check) and include its raw output. Mask every discovered +credential value per your secret-handling rules — file:line plus a 2–4 +character masked preview, never the value itself." ## Triage @@ -29,6 +50,15 @@ Write `analysis/$1/SECURITY_FINDINGS.md`: - Findings table sorted by severity - Dependency CVE table (package, installed version, CVE, fixed version) +If any hardcoded credentials were found, also write +`analysis/$1/SECRETS.local.md` (the gitignored quarantine file from Step 0): +one row per credential — masked preview, `file:line`, credential type, what +it appears to grant access to, production/test guess, and a rotation +recommendation. With `--show-secrets`, append the raw value column here — +this file only. SECURITY_FINDINGS.md gets a one-line pointer: +"N hardcoded credentials found — inventory in SECRETS.local.md (gitignored; +not for sharing)." + ## Remediate For each **Critical** and **High** finding, draft a minimal, targeted fix.