diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 49b5149..9f35dfa 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -18,9 +18,9 @@ change only roadmap audit documents do not change that production target. ## Executive Summary -Pending final synthesis. The initial baseline is healthy, and the architecture -and dependency-boundary review found one Low documentation finding and no -production dependency inversion. +Pending final synthesis. The initial baseline is healthy. The architecture and +configuration/CLI reviews have found one Medium correctness finding and two +Low findings, with no production dependency inversion. ## Finding Index @@ -29,6 +29,8 @@ Final cross-area ordering is pending synthesis. | ID | Severity | Category | Title | | --- | --- | --- | --- | | ARCH-001 | Low | Documentation/Comments | Repair broken ADR cross-references | +| CFGCLI-001 | Medium | Correctness | Reject additional YAML documents | +| CFGCLI-002 | Low | Correctness | Reject a blank command-level LLM profile | ## Findings @@ -57,6 +59,63 @@ Final cross-area ordering is pending synthesis. edits. - **Grouping:** Independent. +### Configuration And CLI Composition + +### CFGCLI-001 — Reject additional YAML documents + +- **Severity:** Medium +- **Category:** Correctness +- **Evidence:** `internal/core/config/file_config.go:327`–`353` + constructs a `yaml.Decoder`, enables `KnownFields`, and calls `Decode` only + once. `yaml.Decoder.Decode` reads the next YAML document, so input such as a + valid version 4 configuration followed by `---` and another configuration is + accepted with the second document ignored. The strict file tests in + `internal/core/config/file_config_contract_test.go` cover malformed values, + unknown fields, and duplicate normalized identifiers, but not an additional + document. +- **Impact:** An operator can append a syntactically valid configuration + document, receive a successful validation result, and then run with only the + first document. Settings in the ignored document—including operational or + pipeline settings—have no effect without a diagnostic, contradicting the + documented strict single-file model. +- **Recommendation:** After decoding `FileConfig`, decode once more and require + `io.EOF`; reject any second document, including an empty or malformed one, + with a contextual configuration error. +- **Preserve:** Keep version gating before the full strict decode, unknown-field + and duplicate-key rejection, and the existing defaults → file → environment + precedence unchanged. +- **Validation:** Add focused parser cases for a second valid document, a + second malformed document, and ordinary trailing whitespace/comments; run + `go test ./internal/core/config ./internal/cli`. +- **Grouping:** Independent. + +### CFGCLI-002 — Reject a blank command-level LLM profile + +- **Severity:** Low +- **Category:** Correctness +- **Evidence:** `internal/cli/run.go:149`–`166` registers `--llm-profile` as a + plain string flag, while the command's presence-aware empty-value checks + cover session ID, reasoning effort, output/debug directories, and recompute + step but not this flag. `runPipelineCommand` passes the resulting string to + `Config.Resolve`; `internal/framework/pipeline/profile.go:1255`–`1277` trims + an empty override and treats it as absent. The run contract tests cover a + valid override and an unknown non-empty profile, but not an explicitly + supplied blank value. +- **Impact:** A shell expansion such as `--llm-profile "$PROFILE"` with an + unset or blank value succeeds by silently using binding, pipeline, or + PromptKit defaults. The run can therefore use a different model/profile than + the operator explicitly intended to select. +- **Recommendation:** Make the CLI flag presence-aware and reject an explicitly + supplied empty or whitespace-only profile ID as command syntax before config + loading or physical-state allocation. +- **Preserve:** Keep a genuinely omitted override optional, trim non-empty IDs, + retain command → binding → pipeline → PromptKit precedence, and continue to + apply overrides only to selected LLM-backed bindings and validators. +- **Validation:** Add command-contract cases for blank and whitespace-only + values that assert exit status 2 and no state allocation, plus retain the + valid and unknown-profile run cases; run `go test ./internal/cli`. +- **Grouping:** Independent. +