فهرست منبع

code-modernization: close remaining credential-leak paths

A red-team pass found four ways credential values still reached
shareable artifacts after the initial redaction:

- the remediation patch: a diff removing a hardcoded secret carries the
  raw value on its '-' lines by construction. harden now splits output:
  non-credential hunks in the shareable security_remediation.patch,
  credential hunks in a gitignored security_remediation.local.patch
  with comment-only placeholders in the shareable file
- the other four agents had no secret-handling rules. legacy-analyst
  (hardcoded-config evidence in tech-debt findings),
  business-rules-extractor (credentials recorded as rule parameters),
  test-engineer (legacy literals becoming committed test fixtures), and
  architecture-critic (quoted code in notes files) now all mask values
  and cite file:line; assess's tech-debt prompt and ASSESSMENT.md
  masking now cover every section, not just Security Findings
- non-git projects: a .gitignore protects nothing under SVN/Mercurial.
  Both commands now refuse --show-secrets without git and write the
  quarantine file to ~/.modernize/<system>/ outside the project tree
- the patch-apply instruction was wrong in both documented layouts
  (symlinked legacy/ broke relative paths). Patches are now written
  with project-root-relative paths and applied from the project root

Also: --show-secrets is now position-independent in both commands, and
the README documents the full model.
Morgan Lunt 3 ماه پیش
والد
کامیت
9d49c4b135

+ 1 - 1
plugins/code-modernization/README.md

@@ -26,7 +26,7 @@ mkdir -p legacy && ln -s /path/to/your/legacy/codebase legacy/billing
 
 ## 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/<system>/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.
+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/<system>/SECRETS.local.md`, which the commands gitignore before writing; on non-git projects the quarantine file goes to `~/.modernize/<system>/` 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
 

+ 6 - 0
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,

+ 9 - 0
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
+`<credential — masked, see file:line>` 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`

+ 9 - 0
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,

+ 9 - 0
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 /

+ 17 - 7
plugins/code-modernization/commands/modernize-assess.md

@@ -5,7 +5,9 @@ argument-hint: <system-dir> [--show-secrets] | --portfolio <parent-dir>
 
 **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,7 +110,9 @@ 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,
@@ -150,14 +154,20 @@ 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,
+   — 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. In ASSESSMENT.md, the Security Findings section lists credential
-   findings with masked values only, plus a one-line pointer:
+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:

+ 1 - 1
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  <trigger>
   Then  <outcome>
   [And  <additional outcome>]
-**Parameters:** <constants, rates, thresholds with their current values>
+**Parameters:** <constants, rates, thresholds with their current values — credentials masked: `<credential — masked, see file:line>`>
 **Edge cases handled:** <list>
 **Suspected defect:** <optional — legacy behavior that looks wrong; decide preserve-vs-fix during transform>
 **Confidence:** High | Medium | Low — <why; if < High, state the exact SME question>

+ 52 - 24
plugins/code-modernization/commands/modernize-harden.md

@@ -3,8 +3,11 @@ description: Security vulnerability scan with a reviewable remediation patch —
 argument-hint: <system-dir> [--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).
@@ -14,19 +17,25 @@ to `analysis/$1/`. The user reviews and applies (or not).
 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.
+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 (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.
+   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
 
@@ -62,23 +71,38 @@ 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.
@@ -87,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`