Validate Heroku application source findings

[amjadsy]

Validate Heroku application source findings

Problem

PR #239 defined the request and findings format for a future Heroku application
source review, but the format alone cannot determine whether a submitted result
is complete, internally consistent, safe to retain, or supported by the reviewed
source files.

Without executable checks, a later source-review phase could accept missing or
duplicate answers, broken references, unsafe file paths, unsupported runtimes,
literal credentials, target recommendations, or findings whose cited files do
not exist.

Solution

Add a deterministic TypeScript validator for Heroku application source findings.
It:

• Selects the required source questions from non-secret Heroku inventory signals.
• Validates requests and findings against their specific contract definitions.
• Enforces one answer per requested question and checks cross-record references.
• Supports the initial Ruby, Java, and Node.js runtime segment.
• Verifies cited files, line ranges, workspace containment, symlink handling, and
  source file and byte limits.
• Rejects literal credentials, connection strings, bearer tokens, and
  destination recommendations.
• Replaces an invalid submission with a complete UNKNOWN result instead of
  retaining partial or optimistic findings.
• Revalidates the assembled review artifact before it can be retained.

The advisor and migrate plugin copies remain byte-identical.

Broader Context

This is the next incremental change after PR #239 in the effort to make the
Heroku migration workflow use application-code evidence when assessing Elastic
Beanstalk. PR #239 defined what source-review output may contain; this PR makes
those rules executable.

A separate follow-up PR will add the read-only LLM source investigation phase
that produces these findings. Later changes will use validated findings for
deterministic Beanstalk compatibility, target selection, reporting, cost
assumptions, and starter-artifact generation.

The existing Elastic Beanstalk, Fargate, and EKS destinations remain in place.

Scope

This PR does not run source investigation or connect the validator to Discover,
Clarify, Design, Estimate, Generate, Feedback, or Workshop. It does not change
destination selection, cost calculations, reports, generated infrastructure, or
current customer-visible migration behavior.

No live Heroku run is required for this PR because the validator is not yet
invoked by a migration phase.

Type of Change

• Bug fix
• New plugin/power/tool
• Enhancement to existing content
• Documentation update
• Guardrail/CI update

Team Folder

• advisor/
• migrate/
• Other: root test-task registration

Validation

• 13 application-source contract tests.
• 34 production-validator tests.
• 126 frontmatter-validator tests across both plugin copies.
• 12 Elastic Beanstalk runtime-setting regression tests.
• TypeScript compilation for both plugin copies.
• Cross-plugin drift check: 281 identical files and 27 allowlisted differences.
• Fixture validation and eight fixture assertions per plugin copy.
• Formatting, Markdown lint, model-ID lint, and shared-file checks.
• Bandit, Semgrep, Gitleaks, Checkov, and Grype security checks.
• Complete mise run build.

Checklist

• I have read the CONTRIBUTING.md guidelines.
• My changes do not include hardcoded secrets, credentials, or internal-only
  content.
• The repository formatting, lint, test, and security gates pass.
• Both published Heroku skill surfaces contain byte-identical validator and
  contract files.
• Existing migration phases and destination behavior remain unchanged.

By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.

[herosjourney]

Reviewed: head 7263c8f (base a72f933 = current origin/main), 15 files (+2219/−270) — heroku-to-aws (references/shared/ contract + schema + README), new tools/application-source-review.ts, node-shims.d.ts, tsconfig.json in both trees; tests + mise.toml registration.
Ran: mise run build (exit 0 locally; the remote build check was still pending at review time), mise run test (contract suite 13/13, review suite 23/23, others green), cmp on every twin pair, git merge-tree vs origin/main (CLEAN), schema keyword audit (every keyword the contract uses is in the hand-rolled validator's supported set; no array-valued type), and a mutation-probe harness against evaluateSubmission / scanDisallowedContent / measureSourceRoot / validateReviewArtifact using schema-valid records.
Did not run: no end-to-end heroku-to-aws run (nothing invokes the validator yet); no Python suites (none in diff).

Cross-plugin parity: PASS — 3 skills/ twin pairs byte-identical, drift:check unchanged (allowlist diff empty). Outside skills/ mirrored by hand: tools/application-source-review.ts, tools/frontmatter-validator/node-shims.d.ts, tsconfig.json all IDENTICAL. Tests exist only in migrate/tests/tools/ — same as the pre-existing application-source-contract.test.ts convention, and the new suite asserts byte-identity of both tools/ copies, so coverage effectively spans both.

Change narrative: #239 shipped a data-only contract → this PR adds the executable validator (draft-07 subset + semantics + credential/target scans + filesystem containment/budgets + fail-closed wholesale UNKNOWN) and folds the contract test's inline validator into it → contract doc, schema (configuration_names pattern tightened), two tools/ copies, tests, mise.toml. Body vs diff: matches, including the explicit "not wired into a phase" deferral. One overclaim: the test file header describes "the read-only source_review phase … phase wiring, fixture state" — there is no phase in this PR.

Axis	Stance	One line
Parsimony	advisory	Dead residue of a stripped publication/CLI layer: 7 unused imports, LIMITS.maxArtifactBytes, publishableUnknown() + 2 unused test imports, shim additions only they needed
Completeness	finding	validateReviewArtifact (~100 lines, the artifact-integrity gate) has zero tests; supported-runtime set exists only in code
Security	clear	scanners green; validator is defensive (realpath containment, per-segment symlink walk, no exec/network, bounded regexes)
Alternatives	advisory	Scope scans to text-typed fields; exempt redaction placeholders — see below
Contradictions	finding	Contract permits "redacted commands"; the credential scan rejects the common redaction shapes → wholesale UNKNOWN
Overlap	clear	Contract test now imports the production validator instead of carrying its own — good consolidation
Cross-plugin parity	clear	see block above
Propagation & data	advisory	SSOT for supported runtimes is SUPPORTED_RUNTIMES in code; contract.md never states it; runtime is free-text name, so Node.js 20 / ruby-3.2 fail closed

Should-fix

1. [Contradictions] Credential scan rejects the redacted commands the contract tells producers to emit. — tools/application-source-review.ts:381-382 vs application-source-contract.md:9 (both trees). Detail inline.

   Probed with schema-valid process_commands records: heroku run rake seed --token=<redacted> → UNKNOWN (both :381 and :382 fire); API_KEY=******** node app.js → UNKNOWN; node app.js → RETAINED. [^\s"']{8,} treats <redacted> and ******** as a secret. Once wired, any producer that redacts the way the contract asks loses the whole application to UNKNOWN.

2. [Completeness] Cost/target heuristics false-fail ordinary source facts. — tools/application-source-review.ts:389 and :386. Detail inline.

   \$\s?\d matches shell positional parameters: bin/start $1 → whole submission UNKNOWN ($PORT is fine). :386's deploy(?:ment)?…{0,80}…amazon rds matches a factual external_services.role of "deployment reads the existing Amazon RDS reporting replica" and a recurring_jobs.coordination of "existing nightly migration of totals into Amazon Aurora" — exactly the existing-dependency description the contract's external_services row asks for. The existing test (application-source-review.test.ts:457) passes because it puts "Amazon RDS" in a provider_reference field alone, on a record shape the schema wouldn't accept.

3. [Completeness §6] validateReviewArtifact has no test. — tools/application-source-review.ts:629-720. Detail inline. Neither test file imports it. It is the gate that stops a controller from swapping accepted findings and carries its own rules (exact key set, request/inventory order match, canonical-UNKNOWN check, RETAINED-cannot-have-limitations). Probes show it works on the happy path and rejects a non-canonical UNKNOWN detail, but nothing in CI pins that. The dead publishableUnknown() helper in the test (:108) looks like the fixture this test was meant to use.

4. [Propagation] Supported runtimes live only in code, and runtime is free text. — tools/application-source-review.ts:55. Detail inline. contract.md never states which runtimes retain and which fail closed, and runtimeFrameworkRecord.runtime is $ref name (any 1–128 chars). Probed: Node.js → RETAINED, Node.js 20 → UNKNOWN, ruby-3.2 → UNKNOWN, python → UNKNOWN. Python being out of scope for a Heroku source reviewer deserves a sentence in the body.

5. [Parsimony] Residue of a stripped publication layer. — tools/application-source-review.ts:10-21, 113; tests/tools/application-source-review.test.ts:1-3, 8-9, 108. Detail inline. Unused: randomUUID, renameSync, unlinkSync, writeFileSync, statSync, basename, dirname (import-only), LIMITS.maxArtifactBytes, test helper publishableUnknown, test imports existsSync/lstatSync. tsc passes because noUnusedLocals is off, so CI won't catch it. Also fix the test header to stop describing a phase that doesn't exist. Both trees.

Nits

6. [Completeness] Build-output directories eat the 5,000-file budget for the very runtimes that are supported. — :482 skips only .git, .migration, node_modules, .venv. Probed: a Ruby app with vendor/bundle (5,001 empty files) → withinLimits: false → whole app UNKNOWN. Java target/ / build/ / .gradle, Node dist/ / .next/ / coverage/, Rails tmp/ / log/ will do the same. Consider extending the skip list or honoring .gitignore. Advisory because the budget is a deliberate design choice.
7. :495 if (dir === rootAbs && entry === ".git") continue; is unreachable — .git is already skipped at :482.
8. contract.md:62 "The validator remains dormant in this PR" — the doc outlives the PR; say "is not yet invoked by any phase".
9. node-shims.d.ts:37 and :42 both declare relative in node:path (harmless overload). SKILL.md:178 still labels the schema "future" in both trees — fine while dormant, but update when the phase lands.
10. validateRequest re-checks configuration_names with the same regex the schema now enforces (^[A-Za-z_][A-Za-z0-9_]*$); the code branch is unreachable after a schema pass. Keep one.

Heads-up (out of diff, pre-existing — not blocking)

• application-source-contract.test.ts has lived only in migrate/tests/tools/ since #239 while frontmatter-validator.test.ts is in both trees. Now that there are two migrate-only suites for shared tooling, worth deciding whether advisor/tests/tools/ should carry them too or whether the byte-identity assertion is the agreed substitute.

Alternatives (max 2)

1. Scope the disallowed-content scans to text-typed fields only (command, role, coordination, purpose, detail, …) and exempt identifiers/names — wins: fewer false-fails on setting_name / service_reference; costs: small walker change. Recommendation: do alongside finding 2.
2. Strip the offending field rather than fail the whole application — wins: keeps 21 good findings when one string trips a heuristic; costs: violates the contract's "no partial acceptance" rule. Recommendation: keep wholesale fail-closed, fix the regexes instead.

Verdict

Comment — nothing kind-checkable rises to Blocking while the validator is dormant, but findings 1 and 2 are contradictions between the contract text and the code that will turn into wholesale UNKNOWNs on ordinary apps the moment the next PR wires it in, and finding 3 leaves the artifact gate unproven.

[herosjourney]

[Should-fix] This rejects the redacted commands application-source-contract.md:9 tells producers to emit.

[^\s"']{8,} treats a placeholder as a secret. Probed with schema-valid process_commands records at 7263c8f:

heroku run rake seed --token=<redacted>   -> UNKNOWN (this line and :382 both fire)
API_KEY=******** node app.js              -> UNKNOWN
node app.js                               -> RETAINED

Once a phase invokes this, any producer that redacts the way the contract asks loses the whole application to UNKNOWN.

Suggested (both lines): exclude placeholder shapes from the value class, e.g.

/\b(?:password|passwd|secret|token|api[_-]?key|access[_-]?key)\s*[:=]\s*["']?(?!<|\*{3,}|\[|\$\{?[A-Z_])[^\s"'<>*\[\]]{8,}/i,

and add --token=<redacted> / KEY=******** as retained-positive tests next to the SIGNING_SECRET case. Same on the advisor/ copy.

[amjadsy]

Fixed in 26b9825. Credential assignment and CLI patterns now exempt standard redaction placeholders, with retained-positive tests for <redacted>, ********, and ${REDACTED} while literal credentials remain rejected.

[herosjourney]

[Should-fix] \$\s?\d matches shell positional parameters, and :386 matches factual existing-dependency text.

Probed with schema-valid records at 7263c8f:

process_commands.command  "bin/start $1"                                         -> UNKNOWN (this line)
process_commands.command  "bundle exec puma -p $PORT"                            -> RETAINED
external_services.role    "deployment reads the existing Amazon RDS reporting replica" -> UNKNOWN (:386)
recurring_jobs.coordination "existing nightly migration of totals into Amazon Aurora"   -> UNKNOWN (:386)

The external_services row of the contract asks for exactly that kind of description. The existing positive test (application-source-review.test.ts:457) passes only because it puts "Amazon RDS" in a bare provider_reference on a record shape the schema would not accept.

Suggested: require currency context for $ and drop deploy(?:ment)? from the :386 trigger list (or scope :386 to a recommending verb):

/\b(?:target|destination|migrat(?:e|ion))\b.{0,80}\b(?:recommend|should|propose)\b.{0,40}\b(?:elastic beanstalk|fargate|amazon rds\b|amazon aurora|elasticache|amazon eks|amazon msk|app runner)\b/i,
/(?:\$\s?\d[\d,]*(?:\.\d{2})?\s*(?:\/|per)\s*(?:mo|month|hr|hour|yr|year)\b|\bmonthly cost\b|\bUSD\b|\bper month\b)/i,

Add $1 and the "existing Amazon RDS" role as retained-positive tests. Same on the advisor/ copy.

[amjadsy]

Fixed in 26b9825. Target checks now require recommendation or explicit target language, and cost checks require cost/rate context. Schema-valid tests retain $1, existing RDS dependency text, and existing Aurora job text while still rejecting explicit target and cost output.

[herosjourney]

[Should-fix] No test imports validateReviewArtifact.

This is the gate that stops a controller from swapping accepted findings, and it carries rules nothing else checks (exact key set, request/inventory order match, canonical-UNKNOWN replacement, RETAINED-cannot-have-limitations). My probes at 7263c8f show the happy path accepts and a non-canonical UNKNOWN detail is rejected, but CI pins none of it. The dead publishableUnknown() helper at application-source-review.test.ts:108 looks like the fixture this was meant to use.

Suggested minimum cases: RETAINED accepted; RETAINED with non-empty limitations rejected; UNKNOWN with a detail outside ALLOWED_UNKNOWN_DETAILS rejected; request differing from expectedRequests[i] rejected; entry with an extra key rejected. Same test proves the advisor/ copy via the byte-identity assertion.

[amjadsy]

Fixed in 26b9825. validateReviewArtifact now has happy-path RETAINED and canonical UNKNOWN coverage plus rejection cases for retained limitations, altered request/order, extra keys, non-canonical UNKNOWN details, and the wrong findings document type.

[herosjourney]

[Should-fix] The supported-runtime set lives only here, and runtime is free text in the schema.

application-source-contract.md never states which runtimes retain and which fail closed, and runtimeFrameworkRecord.runtime is $ref name (any 1–128 chars). Probed at 7263c8f:

"Node.js"     -> RETAINED
"Node.js 20"  -> UNKNOWN  (unsupported runtime: node.js 20)
"ruby-3.2"    -> UNKNOWN
"python"      -> UNKNOWN

So a producer has no documented vocabulary to hit, and a version suffix fails the whole app closed.

Suggested: state the supported set and the exact runtime tokens in contract.md (make it the SSOT), and either give runtime an enum in the schema or normalize before the lookup (/^(node(?:\.?js)?|ruby|java)\b/). Python being out of scope for a Heroku source reviewer deserves a sentence in the PR body. Same on the advisor/ copy.

[amjadsy]

Fixed in 26b9825. The contract and PR body now state the initial Ruby, Java, and Node.js scope and canonical ruby, java, and nodejs labels. Common version-bearing labels such as Ruby 3.3, ruby-3.2, Java 21, and Node.js 20 normalize successfully; other runtimes still fail closed.

[herosjourney]

[Should-fix] Unused imports — residue of the stripped publication layer.

randomUUID, renameSync, unlinkSync, writeFileSync, statSync, basename, dirname are import-only in this file; LIMITS.maxArtifactBytes (:113) is never read; the test has a dead publishableUnknown() helper and unused existsSync / lstatSync imports; the node-shims.d.ts additions for process.pid, renameSync, unlinkSync, randomUUID exist only for them. tsc passes because noUnusedLocals is off, so CI won't flag this.

Suggested: remove them (and the orphaned shim declarations) or land the layer they belong to; and fix the test header, which describes "the read-only source_review phase … phase wiring, fixture state" that this PR does not contain. Same on the advisor/ copy.

[amjadsy]

Fixed in 26b9825. Removed the unused publication-layer imports, constant, and orphaned shim declarations, corrected the test header, and used the former UNKNOWN helper in the new artifact-validation tests.

[herosjourney]

[Nit] Build-output directories for the supported runtimes eat the 5,000-file budget.

Probed at 7263c8f: a Ruby app with vendor/bundle holding 5,001 empty files → withinLimits: false → whole app UNKNOWN. Java target/ / build/ / .gradle, Node dist/ / .next/ / coverage/, Rails tmp/ / log/ will do the same on an ordinary working tree. Consider extending this skip list or honoring .gitignore. Advisory — the budget itself is a deliberate choice.

Also: :495 if (dir === rootAbs && entry === ".git") continue; is unreachable since .git is skipped here.

[amjadsy]

Fixed in 26b9825. Source measurement now excludes common dependency, cache, generated, coverage, log, and temporary directories, including vendor/bundle; excluded paths cannot be cited. Added coverage for the supported-runtime directory examples and removed the unreachable .git branch.

[herosjourney]

[Nit] "in this PR" will be stale the moment this merges.

This reference doc outlives the PR and a skill reader has no PR context. Suggested: "The validator is not yet invoked by any phase." Same on the advisor/ copy.

[amjadsy]

Fixed in 26b9825. Both contract copies now use durable wording: the validator is not yet invoked by any migration phase.

[leon1418]

[🤖 AI review 🤖]

Reviewed 7263c8f17ff2a92f398c90f66b387d30b2175d2b against a72f93343e710a219ba23fbe191d353547abc19d, including both plugin copies and the existing seven review threads. Three distinct P2 validation defects are detailed inline; each reproduces in both copies.

Local validation: all 172 tests across the five registered Node suites pass with Node 24.14.1 and Terraform 1.13.5; both TypeScript checks, formatting, Markdown lint, cross-plugin drift, six changed mirror pairs, and git diff --check pass. The initial runtime-suite attempt used Terraform 1.15.5 and failed its version assertion; the required-toolchain rerun passed. Separate adversarial probes reproduce the reported defects. GitHub CI currently reports all nine checks successful. No migration end-to-end run was performed: this validator is not yet invoked by a phase.

[leon1418]

[🤖 AI review 🤖]

Re-reviewed 26b982515e932c55375e3f3e24e6aebdb4584acc against b30cb39be02d2eae78cb8c482f4018435d4dc988. All three findings from my previous review are verified fixed in both copies; I confirmed each in its original thread. Two new P2 regressions in the directory-exclusion change remain, detailed inline: regular build scripts are rejected, and an excluded directory chosen as the source root can supply retained evidence. Both reproduce through evaluateSubmission and validateReviewArtifact, with the previous head as a control.

Local validation: all 185 tests across the five registered Node suites pass with Node 24.14.1 and Terraform 1.13.5; both TypeScript checks, formatting, Markdown lint, cross-plugin drift, all six changed mirror pairs, and git diff --check pass. All nine GitHub CI checks are successful. No live migration was run; no phase invokes this validator yet.

[leon1418]

[🤖 AI review 🤖]

Re-reviewed ea495a656813c78142cc381933ec4fea7b5cbae7 against ade57adaf9d776d18d316d28d3cee3f746582ced, including both plugin copies, the full discussions, and affected validation consumers. All five of my findings are verified fixed, with no remaining substantive concerns from this review. The two latest fixes are confirmed in their original threads; the earlier wrong-document, unreadable-citation, and undeclared-property cases still fail closed.

Local validation: all 187 registered Node tests pass, plus 42 focused probe scenarios covering both copies and old-head controls. Both TypeScript checks, formatting, Markdown lint, cross-plugin drift, six changed mirror pairs, and git diff --check pass. All nine GitHub CI checks are successful. No live migration was run because no phase invokes this validator yet.

[leon1418]

[🤖 AI review 🤖]

Completed the OCR-assisted, independent general, and repository review of e00763ae2ba8631fcffc4f6ea4583692aafc5282. The five earlier corrections remain intact. I missed one additional credential-retention case in my previous clean review of ea495a6; the affected source is unchanged from that revision.

One P2 correction remains: recognize prefixed secret-variable assignments in both copies of tools/application-source-review.ts, and add shared regression cases in migrate/plugins/migration-to-aws/tests/tools/application-source-review.test.ts. Source-backed DATABASE_PASSWORD and AWS_SECRET_ACCESS_KEY literal assignments must produce canonical UNKNOWN findings and fail RETAINED-wrapper validation. Names-only fields, environment references and redaction placeholders must remain accepted. Keep the mirrors identical and run the focused suites and both type checks. Details and reproduction are inline.

Fresh local validation: 187 registered tests, both type checks, formatting, Markdown lint and mirror/drift checks pass. The independent reviewer also ran the 49-test source-validation subset and its own probes; I reproduced this finding in both copies and at the previous head. GitHub CI reports all nine checks successful. No live migration was run; phase activation remains outside this PR.

[leon1418]

[🤖 AI review 🤖] [P2] Match complete secret-variable names in literal assignments

The word boundary before password/secret/access_key does not match after _. A valid, source-backed process_commands finding containing DATABASE_PASSWORD=syntheticExampleValue123 node app.js therefore returns retained: true with the literal unchanged, and validateReviewArtifact accepts its RETAINED wrapper. AWS_SECRET_ACCESS_KEY=syntheticExampleValue123 node app.js behaves the same way in both copies; a bare PASSWORD=... control is rejected. Recognize secret-bearing complete configuration identifiers, and exercise both entry points in regression tests while preserving names-only fields, ${...} references and redaction placeholders. This case also reproduces at ea495a6 and was missed in my earlier review.

[amjadsy]

Fixed in 8179161. Literal-assignment detection now matches complete underscore-delimited configuration names, including DATABASE_PASSWORD and AWS_SECRET_ACCESS_KEY, in both identical validator copies. Shared regression coverage verifies that source-backed literal assignments produce canonical UNKNOWN findings and fail RETAINED artifact validation, while names-only fields, environment references, and redaction placeholders remain accepted. The complete mise run build passes.

[leon1418]

[🤖 AI review 🤖] [P2] The two illustrated names are fixed at dbd30c2d777e7f315d4d24e411714fb3acedf194: literal DATABASE_PASSWORD and AWS_SECRET_ACCESS_KEY now produce canonical UNKNOWN findings and fail final RETAINED validation in both copies. Safe references and placeholders still pass.

The complete-identifier issue remains for names allowed by configurationNameList (^[A-Za-z_][A-Za-z0-9_]*$): _DATABASE_PASSWORD, DATABASE__PASSWORD, and DATABASE_PASSWORD_ with a long literal are still retained unchanged and accepted by validateReviewArtifact. The new prefix/suffix groups cannot consume leading, repeated, or trailing underscores. Please match the full declared identifier grammar before testing secret-bearing components, and add source-backed boundary cases through both retention entry points. This continues the existing issue; the five earlier fixes remain intact.

[amjadsy]

Fixed in 6a3f49f. The assignment matcher now covers the complete underscore-delimited identifier shape, including leading, repeated, and trailing underscores. Source-backed regression cases exercise both evaluateSubmission and validateReviewArtifact, while names-only fields and environment references remain accepted.

[leon1418]

[🤖 AI review 🤖] Verified fixed at 315133e108320b717de0fe05428ea1fc65d3d5e2 in both copies. _DATABASE_PASSWORD, DATABASE__PASSWORD, DATABASE_PASSWORD_, and the earlier prefixed examples now produce canonical UNKNOWN findings for literal values and fail final RETAINED validation. Names-only inputs, environment references and redaction controls remain accepted in the compared functional cases. The previous dbd30c2 controls reproduce the reported retention gap.

[leon1418]

[🤖 AI review 🤖]

Completed the hybrid follow-up at dbd30c2d777e7f315d4d24e411714fb3acedf194: OCR delegation, an isolated independent general review, repository review, and candidate verification. The original DATABASE_PASSWORD/AWS_SECRET_ACCESS_KEY examples are fixed, and the five earlier corrections remain intact. Two P2 cases remain in the same credential recognizer: the existing full-identifier issue persists for valid underscore boundaries (updated in its original thread), and this fix regresses hyphenated Java property keys (inline).

Consolidated correction: update both copies of tools/application-source-review.ts and the shared migrate/plugins/migration-to-aws/tests/tools/application-source-review.test.ts. Cover the declared underscore identifier grammar and restore hyphenated api-key/access-key detection. Both initial evaluation and final RETAINED validation must reject these literal values while preserving names-only fields, environment references, redactions, and current fixed cases. Keep mirrors identical and run the focused suites and both type checks. No migration-phase integration is required.

Fresh local validation: 188 registered tests and scoped type/lint/format/drift checks pass. The independent reviewer passed 50 authored tests and 12 infrastructure regression tests, and I reproduced both findings against old/current sources in both mirrors. All nine current-head GitHub CI checks pass. No live migration or deployment was executed.

[leon1418]

[🤖 AI review 🤖] [P2] Preserve hyphenated credential keys in Java property assignments

This changes api[_-]?key/access[_-]?key to api_?key/access_?key. A valid Java source finding containing java -Dservice.api-key=syntheticVerifyValue123 -jar app.jar is now retained with its literal credential unchanged, and its RETAINED artifact passes validation. service.access-key behaves the same way. Both were rejected at e00763a, and both current copies reproduce the regression. The CLI pattern does not cover this dot-prefixed property name. Restore the hyphenated alternatives while retaining the new underscore-name handling, and test these source-backed Java commands through both initial and final retention.

[amjadsy]

Fixed in 6a3f49f. Restored hyphenated api-key/access-key assignment detection and added Java -D property cases through both initial evaluation and final artifact validation. Redacted and environment-reference forms remain accepted.

[leon1418]

[🤖 AI review 🤖] Verified fixed at 315133e108320b717de0fe05428ea1fc65d3d5e2 in both copies. Source-backed Java -Dservice.api-key and -Dservice.access-key literal commands are rejected at initial and final retention again, with canonical UNKNOWN replacement. The previous dbd30c2 controls retain them. CLI, environment-reference and redaction controls preserve their intended behavior.

[leon1418]

[🤖 AI review 🤖]

Completed the hybrid follow-up at 315133e108320b717de0fe05428ea1fc65d3d5e2. The two previous findings are verified fixed in both copies and confirmed in their original threads; the five earlier fixes remain intact. Two P2 issues remain: the latest matcher introduces severe backtracking on small valid names-only input, and quoted JSON credential keys still bypass retention checks. The quoted-key case also exists at dbd30c2 and was missed in my previous review.

Consolidated correction: make assignment recognition bounded and quote-aware in both copies of tools/application-source-review.ts, with shared regression coverage in migrate/plugins/migration-to-aws/tests/tools/application-source-review.test.ts. Prefix/suffix underscore near misses within the existing name limit must complete without pathological backtracking. Quoted password/token literals in source-backed commands must produce UNKNOWN and fail final RETAINED validation. Preserve the verified underscore/JVM, names-only, reference, redaction and earlier safety cases. No migration-phase integration is requested.

Fresh validation: 188 registered Node tests and both type/lint/format/drift checks pass; the independent reviewer passed 62 overlapping tests and its own bounded probes. I reproduced both issues in both current copies and against the prior source. All nine GitHub CI checks are successful. The source-investigation phase is still inactive; no live deployment was tested.

[leon1418]

[🤖 AI review 🤖] [P2] Remove overlapping underscore repetitions from credential scanning

The nested underscore groups can partition one run in many ways while looking for an assignment. An otherwise valid request with configuration_names: ["A" + "_".repeat(32) + "PASSWORD"] (41 characters, no value) causes both current entry points to exceed a three-second subprocess limit in both copies; the previous version returns in roughly three milliseconds. A trailing underscore run is also severely slower. These inputs are well inside the existing schema/byte bounds, so a small request blocks the synchronous validator. Use a non-overlapping matcher or bounded assignment tokenization, covering both prefix and suffix runs, and add bounded near-miss tests without narrowing the supported identifier grammar.

[leon1418]

[🤖 AI review 🤖] [P2] Recognize quoted credential keys before retaining command text

A valid source-backed command node app.js --config='{"password":"syntheticVerifyValue123"}' is retained with the literal unchanged, and its RETAINED wrapper passes final validation. Quoted token behaves the same way. The assignment pattern expects colon/equals directly after the sensitive name, so the closing key quote prevents a match. I reproduced this in both copies using all 15 always-requested findings; bare PASSWORD literals are rejected and redaction/reference controls pass. Recognize paired quoted keys in the bounded scanner and cover both entry points. This also reproduces at dbd30c2 and was missed in my earlier review.