diff --git a/plugins/code-modernization/README.md b/plugins/code-modernization/README.md index 1de150cb..44b93dac 100644 --- a/plugins/code-modernization/README.md +++ b/plugins/code-modernization/README.md @@ -29,6 +29,10 @@ The commands degrade gracefully, but each of these makes the output meaningfully - **The whole system in the tree**: deployment descriptors (JCL, CICS definitions, route configs), copybooks/includes, and DDL/schemas. Entry-point detection and data lineage in `/modernize-map` are guesswork without them. - **Production telemetry** (optional): an observability MCP server or batch job logs enable the runtime overlay in `/modernize-assess` and timing annotations on critical paths. +## Secret handling + +Legacy systems routinely contain live credentials, and assessment artifacts get committed and shared. **Every agent in this plugin masks credential values** — findings, rule-card parameters, architecture notes, and test fixtures cite `file:line` with a masked preview (`AKIA****`), never the value. 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; on non-git projects the quarantine file goes to `~/.modernize//` instead. `/modernize-harden` splits its remediation diff so credential-removal hunks (which necessarily contain the raw value) land in a gitignored `security_remediation.local.patch`, never the shareable patch. Pass `--show-secrets` to include raw values in the 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/architecture-critic.md b/plugins/code-modernization/agents/architecture-critic.md index 08ba03b1..cf45ec6a 100644 --- a/plugins/code-modernization/agents/architecture-critic.md +++ b/plugins/code-modernization/agents/architecture-critic.md @@ -29,6 +29,12 @@ For **transformed code**: - Does the test suite actually pin behavior, or just exercise code paths? - What would the on-call engineer need at 3am that isn't here? +## Secret handling (mandatory) + +When a finding quotes code containing a credential, key, token, or +connection string, mask the value (`'Pr0d****'`) and cite `file:line` — +findings get appended verbatim to committed notes files. + ## Output Findings ranked **Blocker / High / Medium / Nit**. Each with: what, where, diff --git a/plugins/code-modernization/agents/business-rules-extractor.md b/plugins/code-modernization/agents/business-rules-extractor.md index 31eca621..c0d0bb21 100644 --- a/plugins/code-modernization/agents/business-rules-extractor.md +++ b/plugins/code-modernization/agents/business-rules-extractor.md @@ -40,6 +40,15 @@ of the technology, skip it. from structure/names), **Low** (ambiguous; needs SME). 6. If confidence < High, write the exact question an SME must answer. +## Secret handling (mandatory) + +Rule parameters sometimes *are* credentials — hardcoded passwords in auth +checks, API keys in partner-service calls, connection strings in batch +routines. Record the **rule**, never the **value**: write the parameter as +`` with at most a 2–4 character +preview. Rule cards flow into briefs and steering decks; a raw credential +in a parameter list is a leak. + ## Output format One "Rule Card" per rule (see the format in the `/modernize-extract-rules` diff --git a/plugins/code-modernization/agents/legacy-analyst.md b/plugins/code-modernization/agents/legacy-analyst.md index b22e5736..a6fd5e6c 100644 --- a/plugins/code-modernization/agents/legacy-analyst.md +++ b/plugins/code-modernization/agents/legacy-analyst.md @@ -32,6 +32,15 @@ and explain it in terms a modern engineer can act on. - **Note what's missing.** Unhandled error paths, TODO comments, commented-out blocks, magic numbers — these are signals about history and risk. +## Secret handling (mandatory) + +Legacy code is full of live credentials, and your findings get copied into +shareable reports. When the evidence for a finding — hardcoded config, +dead code, debt, an interface payload — includes a credential, API key, +token, connection string, or private key, **never reproduce the value**. +Cite `file:line` with a masked preview (`VALUE 'Pr0d****'`, +`password=****`). The finding is the practice, not the value. + ## Output format Default to structured markdown: tables for inventories, Mermaid for graphs, 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/agents/test-engineer.md b/plugins/code-modernization/agents/test-engineer.md index 9f49e883..07057ad0 100644 --- a/plugins/code-modernization/agents/test-engineer.md +++ b/plugins/code-modernization/agents/test-engineer.md @@ -28,6 +28,15 @@ someone thinks it should do) so that a rewrite can be proven equivalent. `@Disabled("pending RULE-NNN")` / `@pytest.mark.skip` / `it.todo()` — never deleted. +## Secret handling (mandatory) + +Never copy credential-like literals — passwords, API keys, tokens, +connection strings — from legacy code into test fixtures. Tests live in +the deliverable codebase and get committed. Substitute clearly-fake values +of the same shape and length and note the substitution in a comment. +Anything a test genuinely needs live (e.g. a real database connection for +a dual-run harness) is read from an environment variable, never inlined. + ## Output Idiomatic tests for the requested target stack (JUnit 5 / pytest / Vitest / diff --git a/plugins/code-modernization/commands/modernize-assess.md b/plugins/code-modernization/commands/modernize-assess.md index 188c2d43..692bb643 100644 --- a/plugins/code-modernization/commands/modernize-assess.md +++ b/plugins/code-modernization/commands/modernize-assess.md @@ -1,11 +1,13 @@ --- 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 mode** against the directory that follows. Otherwise run **Single-system -mode** against `legacy/$1`. +mode** against the system dir. Parse flags positionally-independently: +`--show-secrets` may appear before or after the system dir — the system +dir is the first non-flag token. --- @@ -108,12 +110,16 @@ Spawn three subagents **in parallel**: 2. **legacy-analyst** — "Identify technical debt in legacy/$1: dead code, deprecated APIs, copy-paste duplication, god objects/programs, missing error handling, hardcoded config. Return the top 10 findings ranked by - remediation value, each with file:line evidence." + remediation value, each with file:line evidence. If evidence contains a + credential value, mask it per your secret-handling rules — never quote + it." 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 +147,31 @@ 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 lines + `SECRETS.local.md` and `*.local.patch` (create or append as needed — + the patch pattern is used by `/modernize-harden`; writing both now + means the ignore set is complete from first contact). If the project is a + git repo, verify with `git check-ignore -q analysis/$1/SECRETS.local.md` + — do not write any findings until the check passes. If there is **no + git repo** (check for `.svn`/`.hg`/`CVS` too — a `.gitignore` protects + nothing under another VCS): refuse `--show-secrets` and write + `SECRETS.local.md` to `~/.modernize/$1/` instead of the project tree, + telling the user where it went and why. +2. Write `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. Masking applies to **every section of ASSESSMENT.md**, whichever agent + produced the finding — the Technical Debt section quotes hardcoded + config; those quotes follow the same masking rule as Security Findings. + The Security Findings section adds 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-extract-rules.md b/plugins/code-modernization/commands/modernize-extract-rules.md index 1fe7979c..e842867c 100644 --- a/plugins/code-modernization/commands/modernize-extract-rules.md +++ b/plugins/code-modernization/commands/modernize-extract-rules.md @@ -46,7 +46,7 @@ Merge the three result sets. Deduplicate. For each distinct rule, write a When Then [And ] -**Parameters:** +**Parameters:** `> **Edge cases handled:** **Suspected defect:** **Confidence:** High | Medium | Low — diff --git a/plugins/code-modernization/commands/modernize-harden.md b/plugins/code-modernization/commands/modernize-harden.md index b8f7a72c..64e36f3f 100644 --- a/plugins/code-modernization/commands/modernize-harden.md +++ b/plugins/code-modernization/commands/modernize-harden.md @@ -1,14 +1,42 @@ --- 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 -them, and produce a reviewable patch for the critical ones. +Run a **security hardening pass** on the legacy system: find +vulnerabilities, rank them, and produce a reviewable patch for the +critical ones. Parse arguments flag-independently: the system dir +(referred to as `$1` below) is the first non-flag token in `$ARGUMENTS`; +`--show-secrets` may appear anywhere. 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 lines + `SECRETS.local.md` and `*.local.patch`. Create the file or append the + missing lines. +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. +3. **If there is no git repo** (check for `.svn`/`.hg`/`CVS` too — a + `.gitignore` protects nothing under another VCS): refuse + `--show-secrets`, and write `SECRETS.local.md` and any `.local.patch` + file to `~/.modernize/$1/` instead of the project tree, telling the + user where they went and why. + +All secret values in every shareable artifact this command produces are +**masked** (`AKIA****`, `password=****`) and cited by `file:line`. Raw +values may appear in exactly two places, both gitignored: the +`*.local.patch` remediation hunks (unavoidably — see Remediate) and, only +with `--show-secrets`, `SECRETS.local.md`. Never in SECURITY_FINDINGS.md +or patch commentary. + ## Scan Spawn the **security-auditor** subagent: @@ -20,7 +48,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,26 +59,50 @@ 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. -Do **not** edit `legacy/` — write all fixes as a single unified diff to -`analysis/$1/security_remediation.patch`, with a comment line above each -hunk citing the finding ID it addresses (`# SEC-001: parameterize the query`). +Do **not** edit `legacy/` — write fixes as unified diffs with **paths +relative to the project root** (`legacy/$1/...`), applied from the project +root, with a comment line above each hunk citing the finding ID it +addresses (`# SEC-001: parameterize the query`). + +**Credential findings split into two files.** A diff that removes a +hardcoded secret necessarily contains the raw value on its `-` and +context lines — that cannot go in the shareable patch: + +- `analysis/$1/security_remediation.patch` (shareable) — every + non-credential hunk, plus for each credential finding a comment-only + placeholder: `# SEC-NNN: credential remediation — hunk in + security_remediation.local.patch (gitignored; not for sharing)`. +- `analysis/$1/security_remediation.local.patch` (gitignored in Step 0) — + the real, applyable hunks for credential findings only. Add a **Remediation Log** section to SECURITY_FINDINGS.md mapping each -finding ID → one-line summary of the proposed fix and the patch hunk that -implements it. +finding ID → one-line summary of the proposed fix and which patch file +carries the hunk. ## Verify -Spawn the **security-auditor** again to **review the patch** against the -original code: +Spawn the **security-auditor** again to **review both patches** against +the original code: -"Review analysis/$1/security_remediation.patch against legacy/$1. For each +"Review analysis/$1/security_remediation.patch and +analysis/$1/security_remediation.local.patch against legacy/$1. For each hunk: does it fully remediate the cited finding? Does it introduce new -vulnerabilities or change behavior beyond the fix? Return one verdict per -hunk: RESOLVES / PARTIAL / INTRODUCES-RISK, with a one-line reason." +vulnerabilities or change behavior beyond the fix? Confirm no raw +credential values appear anywhere in the shareable patch. Return one +verdict per hunk: RESOLVES / PARTIAL / INTRODUCES-RISK, with a one-line +reason." Add a **Patch Review** section to SECURITY_FINDINGS.md with the verdicts. If any hunk is PARTIAL or INTRODUCES-RISK, revise the patch and re-review. @@ -57,8 +111,12 @@ If any hunk is PARTIAL or INTRODUCES-RISK, revise the patch and re-review. Tell the user the artifacts are ready: - `analysis/$1/SECURITY_FINDINGS.md` — findings, remediation log, patch review -- `analysis/$1/security_remediation.patch` — review, then apply if appropriate - with `git -C legacy/$1 apply ../../analysis/$1/security_remediation.patch` +- `analysis/$1/security_remediation.patch` — review, then apply **from the + project root**: `git apply analysis/$1/security_remediation.patch` + (if `legacy/$1` is a symlink, use `git apply --unsafe-paths` or apply + with `patch -p0` from the project root) +- `analysis/$1/security_remediation.local.patch` — the credential fixes; + apply the same way, and rotate the affected credentials regardless - Re-run `/modernize-harden $1` after applying to confirm resolution Suggest: `glow -p analysis/$1/SECURITY_FINDINGS.md`