feat(cli): protocols subcommand — coverage catalog table + JSON (STORY-152)#353
Conversation
… subcommand (mod story_152)
21 tests in mod story_152 (tests/integration_tests.rs):
- 19 FAIL (all protocols-subcommand assertions) — clap exits 2 "unrecognized subcommand 'protocols'"
- 2 PASS as regression guards:
test_BC_2_12_022_analyze_unaffected (existing analyze behavior is unchanged)
test_BC_2_12_022_mutually_exclusive_flags_error (expects non-zero exit; wrong reason in Red-Gate)
Covers BC-2.12.022 v1.0, BC-2.18.001 v1.4, BC-2.18.002 v1.1.
DF-CANONICAL-FRAME-HOLDOUT-001 canonical values cited per spec:
GOOSE 0x88B8=35000 (IEC 61850-8-1 §4; IEEE RA), POWERLINK 0x88AB=34987
(IEEE RA; Wireshark ETHERTYPE_EPL_V2), BACnet/IP UDP 47808 (ASHRAE 135-2016 Annex J §J.2.1),
Modbus/TCP TCP 502 (IANA/Modbus App Protocol v1.1b3 §4.3.1).
…utput
Implements AC-152-001 through AC-152-008 (BC-2.12.022 v1.0, BC-2.18.001 v1.4,
BC-2.18.002 v1.1; ADR-012 Decisions 3 + 7).
src/cli.rs:
- Add `ProtocolFilter` enum (All/Supported/Unsupported)
- Add `Commands::Protocols { all, supported, unsupported }` variant with
clap `conflicts_with_all` mutual-exclusion (BC-2.12.022 Invariant 2)
src/main.rs:
- Add `Commands::Protocols` dispatch arm (BC-2.12.022 PC-2); deref &bool
destructures; unused `all` elided via `..`
- `run_protocols(filter, json)`: routes to terminal or JSON renderer;
no StreamDispatcher/analyzer calls (architecture compliance)
- `is_protocol_supported()`: derives supported status via
`supported_protocols()` membership; ARP special case explicit
- `render_protocols_terminal()`: Name/Category/Transport/Port(s)/
EtherType/Supported table; [L2] indicator for LinkLayer entries;
0xHHHH (DDDDD) EtherType display; port-102 collision footnote
(conditional on S7comm/S7comm-plus/IEC 61850 MMS/ICCP-TASE.2 presence);
L2/gap-report note (conditional on any LinkLayer entries present)
- `render_protocols_json()`: {"protocols":[...]} per BC-2.18.002 schema;
category "ICS"/"IT" only (ADR-012 Decision 7 — no "L2"); transport
"TCP"/"UDP"/"LinkLayer"; canonical_ports []; ethertype int|null
DF-CANONICAL-FRAME-HOLDOUT-001 verified: GOOSE 0x88B8 (35000),
POWERLINK 0x88AB (34987), BACnet/IP UDP 47808, Modbus/TCP TCP 502.
tests/cli_story_086_tests.rs, tests/cli_story_096_tests.rs:
- Add `Commands::Protocols { .. } => panic!()` arms to non-exhaustive
match blocks (regression guard; BC-2.12.022 Invariant 7)
Gates: cargo test --all-targets ALL GREEN (85 suites, 0 failures, 21/21
story_152 tests); cargo clippy --all-targets -- -D warnings CLEAN;
cargo fmt --check CLEAN.
…es four, coverage tests F-1 (DF-GREEN-DOC-TENSE-SWEEP): Remove all RED-GATE prose from mod story_152. - Module header: drop RED-GATE section (lines 8-17); replace with green-tense regression-guard description. - 17 identical "RED-GATE FAIL: clap rejects..." docstrings replaced via replace_all with "Regression guard: fails if the protocols subcommand is removed or stops exiting 0." - 3 unique RED-GATE NOTE / RED-GATE FAIL docstrings rewritten in green tense. - grep before: 25 occurrences; grep after: 0. - Sibling files (cli_story_086_tests.rs, cli_story_096_tests.rs): not touched by STORY-152 work; no stale tense introduced there. F-3 (MEDIUM): Port-102 footnote now names all four protocols. - src/main.rs: footnote changed to "NOTE: TCP/102 hosts S7comm, S7comm-plus, IEC 61850 MMS, and ICCP/TASE.2 — gap reports on port 102 cannot be attributed to a single protocol." (satisfies AC-152-004, BC-2.18.001 PC-6). - Row-count hardening: test_BC_2_18_001_all_row_count and test_BC_2_12_022_protocols_supported_filter now exclude lines starting with "NOTE:" before counting data rows, preventing footnote inflation. - Footnote-names hardening: test_BC_2_18_001_port102_footnote_names_all_four now locates the "NOTE: TCP/102" footnote LINE specifically and asserts all four names appear IN THAT LINE (not just anywhere in stdout via data rows). New coverage tests added to mod story_152: - test_BC_2_12_022_protocols_spurious_positional_error (EC-152-4): `protocols somefile.pcap` exits non-zero (clap rejects unexpected positional arg). - test_BC_2_18_002_arp_json_canonical (EC-152-10): ARP in --supported --json has transport="LinkLayer", ethertype=null, canonical_ports=[], supported=true. - test_BC_2_12_022_default_equals_all (Invariant 3): no-flag invocation prints same 30 data rows as --all; NOTE: lines excluded from count. Strengthened existing tests: - test_BC_2_12_022_mutually_exclusive_flags_error: now asserts stderr contains "cannot be used with" (clap conflict error), not just any non-zero exit. Gates: cargo test --all-targets ALL GREEN (24 story_152 tests pass, full regression clean), cargo clippy -D warnings clean, cargo fmt --check clean.
…SON declaration-order test LOW-1 (DF-CANONICAL-FRAME-HOLDOUT-001 audit hygiene): Replace the garbled two-line arithmetic ending in "= wait" at tests/integration_tests.rs:455-456 with a single correct derivation line: 0x88B8 = 32768 + 2048 + 176 + 8 = 35000 (IEC 61850-8-1 §4; IEEE RA "IEC GOOSE") DF-SIBLING-SWEEP-001: full-file grep for "= wait", scratchpad, and self-correcting arithmetic remnants — zero found after the fix. LOW-2 (BC-2.18.002 Invariant 1 / PC-4): Add test_BC_2_18_002_json_declaration_order inside mod story_152: runs `protocols --json --all`, parses the "protocols" array, extracts the name sequence, and asserts equality with all_protocols() declaration order.
Security Review — Cycle 1 Triage (STORY-152 / PR #353)
Verdict: CLEAN — CRITICAL 0 | HIGH 0 | MEDIUM 0 | LOW 0 | INFO 2 (both non-blocking, non-security) No security findings require remediation before merge. CWE coverage checked: CWE-88 (arg injection), CWE-134 (format string), CWE-116 (JSON encoding), CWE-22 (path traversal), CWE-78 (OS cmd injection), CWE-400 (resource exhaustion), OWASP A06 (supply chain). All CLEAN. |
PR Review — Cycle 1 Triage (STORY-152 / PR #353)
Verdict: APPROVE — 0 blocking, 0 non-blocking, 1 cosmetic All ACs verified: BC-2.12.022 / BC-2.18.001 / BC-2.18.002. Canonical-frame values verified (DF-CANONICAL-FRAME-HOLDOUT-001): GOOSE 0x88B8=35000, POWERLINK 0x88AB=34987, BACnet UDP 47808, Modbus TCP 502. Exhaustive-match panic! arms in cli_story_086/096 correctly explained as enum blast-radius fallout. Convergence cycle 1 of max 10 — APPROVE (0 blocking findings). Ready for merge gate. |
…VENANCE-CLI-DOC-001 STORY-152 (Wave 68, protocols subcommand): - PR #353 created, AI review APPROVE (0 blocking, 1 cosmetic), security CLEAN (0 CRITICAL/HIGH/MEDIUM/LOW; 2 INFO deferred), CI 11/11 GREEN - Branch tip d34a05f→c4b14f7 via doc-comment-only BC-ID strip (3 lines src/cli.rs) — per-story adversarial convergence on d34a05f REMAINS VALID on c4b14f7 - Pipeline PAUSED at Wave-68 human merge gate; develop unchanged (b285feb) New process-gap PG-HELP-PROVENANCE-CLI-DOC-001 [S-7.02]: - clap /// doc-comments MUST NOT contain internal factory IDs (BC-/VP-/SS-/ADR-) - Codify in implementer green-step checklist + adversary CLI-story axes - Apply to STORY-154 BEFORE first push; route to cycle-close lessons STATE.md updates: - current_step / EXACT RESUME POINT / Session Resume Checkpoint refreshed to MERGE GATE - D-368 added to Decisions Log - PG-HELP-PROVENANCE-CLI-DOC-001 added to Open Items / Backlog - Current Phase Steps: D-368 row prepended; D-363 row archived (now 5: D-368..D-364) - Phase Progress F4 row updated to IN PROGRESS (D-368), PR #353 READY-TO-MERGE - code-delivery/STORY-152/ artifacts included Count-propagation sweep: STATE.md only document with D-368; no index-level count fields changed (stories_delivered=96 unchanged; PR #353 pending merge, develop HEAD b285feb unchanged).
… STORY-152 v1.6 STATE.md updates (D-369): - develop_head updated b285feb → 5c4437a (PR #353 squash-merged) - current_step: Wave-68 wave-level adversarial split verdict + F-W68-01 fix in progress - EXACT RESUME POINT + Session Resume Checkpoint refreshed to 5c4437a - Phase Progress F4 row: split verdict recorded, fix branch noted - Current Phase Steps: D-369 (IN PROGRESS) added; D-364 archived (oldest) - D-369 decision row added - STORY-152-GLOBAL-FLAG-NOOP-001 → RESOLVED-BY-FIX (D-369) STORY-152 → v1.6 (already edited, now committed): - AC-152-002 reverses v1.5 stdout-only reconcile to honor-path + reject-csv - EC-152-11/12/13 added (path routing, --output-format json, csv rejection) - Task 1 test-name citations synced (DF-AC-TEST-NAME-SYNC-001) - No BC content change (BC-2.18.002 PC-1 bare-json→stdout unchanged) Count-propagation sweep: develop_head updated in STATE.md (frontmatter + metadata table + EXACT RESUME POINT + Session Resume Checkpoint + Notes section). No BC/VP/ story counts changed. Remaining b285feb references in D-364 through D-368 rows are intentional historical anchors (immutable audit trail) — preserved verbatim.
…1) (#354) fix(cli): protocols honors --json=PATH; reject --csv (wave-68 F-W68-01) **Wave:** 68 fix-pr-delivery **Finding:** F-W68-01 — `wirerust protocols --json=<PATH>` silent failure **Related Story:** STORY-152 (merged PR #353) **Branch:** `fix/protocols-json-output-routing` @ `4b101ee` **Base:** `develop` @ `5c4437a`      --- ## What was wrong (F-W68-01) `wirerust protocols --json=<PATH>` silently ignored the file path argument and printed JSON to stdout instead of writing the file. This was a SOUL silent-failure violation and a divergence from the `analyze` and `summary` subcommands, both of which correctly route `--json=<PATH>` through the shared `resolve_format` / `write_output` pipeline. Additionally, `--csv` and `--output-format csv` were no-ops: they were parsed by clap but silently fell back to terminal table output with no error and exit code 0. This masked a format mismatch from the operator's perspective. **Root cause:** `run_protocols` took a raw `json: bool` parameter derived from `cli.json.is_some()` before calling the function. This stripped the path information and bypassed `write_output` entirely. The `resolve_format` / `write_output` pipeline that `run_analyze` and `run_summary` use was never wired into `run_protocols`. --- ## The fix Route `run_protocols` JSON output through the same `resolve_format` / `write_output` pipeline used by `run_analyze` and `run_summary`: - Changed `run_protocols(filter, json: bool)` → `run_protocols(filter, cli: &Cli) -> Result<()>` - `resolve_format(cli)` now determines the output mode (JSON, CSV, or default terminal) - `render_protocols_json` now returns a `String` (previously printed to stdout directly) - `write_output(&json_str, cli)` routes to file when `--json=<PATH>`, or to stdout when bare `--json` / `--output-format json` - CSV is explicitly rejected: `--csv` / `--output-format csv` prints an error to stderr and exits non-zero (`std::process::exit(1)`) - The `analyze` and `summary` subcommands are **unchanged** --- ## Behavior matrix | Invocation | Before fix | After fix | |------------|-----------|-----------| | `protocols` (no flags) | terminal table to stdout | terminal table to stdout (unchanged) | | `protocols --json` | JSON to stdout | JSON to stdout | | `protocols --json=<PATH>` | **BUG**: JSON to stdout, no file | **FIXED**: JSON written to `<PATH>`, stdout empty | | `protocols --output-format json` | **BUG**: terminal table (silently ignored) | **FIXED**: JSON to stdout | | `protocols --csv` | **BUG**: terminal table (silently ignored) | **FIXED**: error on stderr, exit non-zero | | `protocols --output-format csv` | **BUG**: terminal table (silently ignored) | **FIXED**: error on stderr, exit non-zero | --- ## Architecture Changes ```mermaid graph TD CLI["src/main.rs:main()<br/>Commands::Protocols"] RP["run_protocols(filter, &cli)<br/>CHANGED: takes &Cli, returns Result"] RF["resolve_format(&cli)<br/>shared pipeline (unchanged)"] RPJ["render_protocols_json()<br/>CHANGED: returns String"] WO["write_output(&str, &cli)<br/>shared pipeline (unchanged)"] RPT["render_protocols_terminal()<br/>unchanged"] CSV_ERR["eprintln! + process::exit(1)<br/>CSV rejection (NEW)"] FILE["<PATH> file"] STDOUT["stdout"] CLI -->|"&cli"| RP RP --> RF RF -->|"Some(Json)"| RPJ RPJ --> WO WO -->|"path present"| FILE WO -->|"path absent"| STDOUT RF -->|"Some(Csv)"| CSV_ERR RF -->|"None"| RPT RPT --> STDOUT style RP fill:#FFD700 style RPJ fill:#FFD700 style CSV_ERR fill:#90EE90 style FILE fill:#90EE90 style WO fill:#D3D3D3 style RF fill:#D3D3D3 ``` **Files changed (exactly 2):** - `src/main.rs` — +55 / -18 - `tests/integration_tests.rs` — +130 (3 new tests appended to `mod story_152`) --- ## Story Dependencies ```mermaid graph LR S151["STORY-151<br/>merged PR #351<br/>src/protocols.rs catalog"] S152["STORY-152<br/>merged PR #353<br/>protocols subcommand"] FIX["FIX-W68-01<br/>this PR<br/>JSON routing fix"] S154["STORY-154<br/>pending<br/>depends on S152"] S151 --> S152 S152 --> FIX FIX --> S154 style FIX fill:#FFD700 style S152 fill:#90EE90 style S151 fill:#90EE90 style S154 fill:#D3D3D3 ``` **Dependency status:** STORY-151 (PR #351) and STORY-152 (PR #353) are both merged to `develop`. --- ## Spec Traceability ```mermaid flowchart LR BC["BC-2.12.022 v1.6<br/>CLI Dispatch + --json routing"] BC --> AC_PATH["AC: --json=PATH writes file<br/>(F-W68-01 fix)"] BC --> AC_STDOUT["AC: --json / --output-format json<br/>writes stdout"] BC --> AC_CSV["AC: --csv / --output-format csv<br/>exits non-zero"] AC_PATH --> T1["test_BC_2_12_022_json_path_writes_file"] AC_STDOUT --> T2["test_BC_2_12_022_output_format_json"] AC_CSV --> T3["test_BC_2_12_022_csv_rejected"] T1 --> Impl["src/main.rs<br/>run_protocols + write_output"] T2 --> Impl T3 --> Impl ``` **Note:** BC-2.12.022 spec-sync (the `json: bool` model in the spec is stale vs the runtime `Option<PathBuf>` reality) is a MEDIUM finding tracked as a phase-5 item. The human decision was "no BC content change" — this is deferred and does NOT block this fix PR. See "Deferred Items" section below. --- ## Test Evidence | Metric | Value | Threshold | Status | |--------|-------|-----------|--------| | New regression-guard tests | 3/3 pass | 100% | PASS | | Full suite (`cargo test --all-targets`) | all green | 100% | PASS | | Clippy (`-D warnings`) | 0 warnings | 0 | CLEAN | | fmt (`--check`) | clean | clean | CLEAN | ### New Tests (This PR) | Test | Covers | What it asserts | |------|--------|----------------| | `test_BC_2_12_022_json_path_writes_file` | `--json=<PATH>` routing fix | File created at path; valid JSON with 30-entry `protocols` array; stdout is empty | | `test_BC_2_12_022_output_format_json` | `--output-format json` fix | Exit 0; stdout is valid JSON with top-level `protocols` array | | `test_BC_2_12_022_csv_rejected` | CSV rejection (both `--csv` and `--output-format csv`) | Exit non-zero; stderr contains "csv"; covers both flag variants | All 3 tests live in `mod story_152` in `tests/integration_tests.rs`, appended after the 25 existing STORY-152 tests (no changes to existing tests). --- ## Demo Evidence Fix-pr-delivery flow: visual demo is not applicable for a targeted behavioral fix (file-write path / error path). The 3 integration tests serve as machine-verifiable evidence replacing manual recording for this fix. The original STORY-152 demo evidence (`docs/demo-evidence/STORY-152/`) is unaffected and remains on the feature branch. --- ## Holdout Evaluation N/A — fix-pr-delivery flow. No wave-level holdout gate for targeted behavioral fixes. Prior STORY-152 wave holdout (VP-041) is unchanged. --- ## Adversarial Review | Pass | Context | P0/CRITICAL/HIGH | MEDIUM | Status | |------|---------|-----------------|--------|--------| | 1 | Fresh context on `4b101ee` | 0 | 0 | CLEAN — APPROVE | **Verdict: CLEAN (0 P0/CRITICAL/HIGH).** Known non-blocking: MEDIUM BC-2.12.022 spec-sync (json:bool model stale) is DEFERRED to phase-5 per human "no BC content change" decision. --- ## Security Review Security analysis dispatched as part of this PR lifecycle — results in Security Review section below. --- ## Risk Assessment ### Blast Radius - **Systems affected:** `src/main.rs` only (function signature + dispatch logic within `run_protocols`) - **`analyze` subcommand:** UNCHANGED — no shared mutable state, no shared function modified - **`summary` subcommand:** UNCHANGED - **Protocol catalog (`src/protocols.rs`):** UNCHANGED - **Tests:** 3 new tests appended to existing `mod story_152`; 0 existing tests modified - **User impact:** Additive fix — `--json=PATH` now correctly writes a file (was broken); CSV now errors explicitly (was silent no-op). Only `protocols` subcommand behavior changes. - **Data impact:** None. Read-only catalog lookup. - **Risk Level:** LOW ### Performance Impact | Metric | Notes | |--------|-------| | Binary size | Negligible — removed one `println!`, added one `String` return | | Runtime | `write_output` path adds one `fs::write` call for `--json=<PATH>`; sub-millisecond | | `analyze` / `summary` paths | Completely unchanged | --- ## Deferred Items ### MEDIUM: BC-2.12.022 spec-sync (json:bool model stale) The BC-2.12.022 behavioral contract models `--json` as a boolean flag (`json: bool`). The runtime CLI parser actually exposes `cli.json: Option<PathBuf>` (an optional path). This model divergence is a **spec documentation gap** — the implementation correctly uses `Option<PathBuf>` throughout. The human decision is that no BC content change is needed at this time. This is tracked for phase-5 spec-evolution as a targeted BC update. **This does NOT block merge.** The fix addresses the observable behavioral failure. --- ## AI Pipeline Metadata <details> <summary>Pipeline Details</summary> ```yaml ai-generated: true pipeline-mode: fix-pr-delivery factory-version: "1.0.0-rc.21" pipeline-stages: tdd-fix: completed (3 new tests, src/main.rs routing fix) adversarial-review: completed (1 fresh-context pass, CLEAN on 4b101ee) security-review: dispatched as part of PR lifecycle convergence: pre-converged (fix adversarially reviewed CLEAN before PR creation) convergence-metrics: adversarial-passes: 1 blocking-findings-at-convergence: 0 models-used: builder: claude-sonnet-4-6 adversary: claude-sonnet-4-6 (fresh-context pass) generated-at: "2026-07-03" ``` </details> --- ## Pre-Merge Checklist - [ ] All CI status checks passing - [x] 3/3 new regression-guard tests green (`test_BC_2_12_022_json_path_writes_file`, `test_BC_2_12_022_output_format_json`, `test_BC_2_12_022_csv_rejected`) - [x] Full `cargo test --all-targets` regression clean - [x] `cargo clippy --all-targets -- -D warnings` clean - [x] `cargo fmt --check` clean - [x] Adversarial review CLEAN (0 P0/CRITICAL/HIGH on `4b101ee`) - [x] Diff limited to exactly 2 files (`src/main.rs`, `tests/integration_tests.rs`) - [x] `analyze` and `summary` subcommands unchanged - [x] STORY-152 (PR #353) merged — upstream dependency satisfied - [x] AI PR review (pr-reviewer) — dispatched as part of this lifecycle - [x] Security review (security-reviewer) — dispatched as part of this lifecycle - [ ] Human approval for squash merge
…ad-code, test hardening (#356) ## Fix: F6 Pre-Hardening Cleanup — 4 LOW/non-blocking F5 adversarial findings **Source:** Phase F5 scoped-adversarial review (feature-protocol-coverage / E-21) **Phase:** F6 pre-hardening cleanup **Severity:** LOW (all 4 findings non-blocking) **Stories:** STORY-152, STORY-153, STORY-154 **Mode:** feature (fix-PR delivery — no demo recording; no user-facing behavior change)    This PR resolves 4 LOW-severity findings carried from the F5 scoped-adversarial review pass against the feature-protocol-coverage epic (E-21, STORY-152/153/154). Changes are internal only: a stale doc-comment correction, a dead-code removal with proof comment, a new integration test for an untested `--all --coverage-gaps` combination, and tightened assertions on 3 weak `contains("unknown")` checks. No public API or user-observable behavior is altered. --- ## Architecture Changes ```mermaid graph TD main["src/main.rs"] tests["tests/integration_tests.rs"] doc_fix["doc-comment corrected\n(STORY-153-RUNANALYZE-DOC-STALE-001)"] dead_code["ARP dead clause removed\n(STORY-154-LOOKUP-ARP-DEADCLAUSE-001)"] new_test["new combined --all --coverage-gaps test\n(STORY-154-ALL-COVERAGEGAPS-TEST-001)"] tightened["3 weak assertions tightened\n(STORY-152/154-WEAK-UNKNOWN-ASSERT-001)"] main --> doc_fix main --> dead_code tests --> new_test tests --> tightened style doc_fix fill:#87CEEB style dead_code fill:#87CEEB style new_test fill:#90EE90 style tightened fill:#90EE90 ``` No architecture changes. All edits are cosmetic (doc-comment), provably-dead-code removal, and test hardening within existing modules. --- ## Story Dependencies ```mermaid graph LR S152["STORY-152\n✅ merged #353"] --> FIX["fix/f6-hardening-cleanup\n🔨 this PR"] S153["STORY-153\n✅ merged #352"] --> FIX S154["STORY-154\n✅ merged #355"] --> FIX style FIX fill:#FFD700 ``` All upstream stories are already merged into `develop`. No downstream blockers. --- ## Spec Traceability ```mermaid flowchart LR BC153["BC-2.05.010\nrun_analyze coverage_gaps param"] BC154["BC-2.12.023\nCoverageGapsSummary invariants"] BC154b["BC-2.12.024\nprotocol gap state"] F1["STORY-153-RUNANALYZE-\nDOC-STALE-001"] F2["STORY-154-LOOKUP-\nARP-DEADCLAUSE-001"] F3["STORY-154-ALL-\nCOVERAGEGAPS-TEST-001"] F4["STORY-152/154-WEAK-\nUNKNOWN-ASSERT-001"] BC153 --> F1 BC154 --> F2 BC154 --> F3 BC154b --> F4 F1 --> SRC1["src/main.rs:193\ndoc-comment corrected"] F2 --> SRC2["src/main.rs:1063\nARP disjunct removed"] F3 --> SRC3["tests/integration_tests.rs\ntest_BC_2_12_023_all_with_coverage_gaps_combination()"] F4 --> SRC4["tests/integration_tests.rs\n3 assertions tightened (lines ~1574,1606,1647)"] ``` --- ## What Changed ### Finding 1: STORY-153-RUNANALYZE-DOC-STALE-001 (commit `7fbb57c`) **File:** `src/main.rs` — `run_analyze()` doc-comment on `coverage_gaps` param **Before:** Stale scaffold comment described the parameter as a pre-ship stub wired to `false` from main() pending STORY-154. **After:** Comment now accurately describes the shipped behavior — the param activates the per-packet UDP gap counter and appends `CoverageGapsSummary` output (AC-154-002/003/007; ADR-012 Decision 9), wired via `*coverage_gaps` from `Commands::Analyze` destructure. --- ### Finding 2: STORY-154-LOOKUP-ARP-DEADCLAUSE-001 (commit `0fdaa29`) **File:** `src/main.rs` — `lookup_protocol_state()` match guard **Removed:** `|| p.name == "ARP"` disjunct in the `Some(p) if ...` guard. **Proof of death:** The enclosing `find()` predicate filters on `p.transport == catalog_transport` where `catalog_transport` is `Transport::Tcp` or `Transport::Udp` (from the packet's network layer). The ARP catalog entry has `transport = Transport::LinkLayer` and `canonical_ports = []`, so it can never pass the `find()` predicate and be returned as `Some(p)`. The `|| p.name == "ARP"` clause in the match guard was therefore unreachable on all possible inputs. A proof comment documents this reasoning in-place. --- ### Finding 3: STORY-154-ALL-COVERAGEGAPS-TEST-001 (commit `abc048e`) **File:** `tests/integration_tests.rs` — new test `test_BC_2_12_023_all_with_coverage_gaps_combination()` **Added:** Integration test covering the `analyze --all --coverage-gaps` combination, which was previously untested. Verifies (1) exit 0, (2) `CoverageGapsSummary` section present, (3) TCP/9600 row shows state `unknown`. The `--all` and `--coverage-gaps` flags are orthogonal (clap config); the test guards against any future clap conflict or orthogonality regression. --- ### Finding 4: STORY-152/154-WEAK-UNKNOWN-ASSERT-001 (commit `f90dfb8`) **File:** `tests/integration_tests.rs` — 3 assertions tightened **Before:** Three tests used `stdout.contains("unknown")` — a substring match that would pass if "unknown" appeared anywhere in stdout for any reason. **After:** Each assertion now uses a line-level check: ```rust let tcp<port>_row_is_unknown = stdout .lines() .any(|l| l.contains("TCP/<port>") && l.ends_with("unknown")); ``` This ties the port identity to the state value, preventing false passes from incidental "unknown" substrings in error messages, headers, or other output rows. Applies to: - TCP/9600 state check (BC-2.12.024 PC-4) - TCP/47808 state check (BC-2.12.024 EC-009 — BACnet/IP transport mismatch) - TCP/53 state check (BC-2.12.024 EC-010 / STORY-154-DNS53-TCP-GAP-001) --- ## Test Evidence | Metric | Value | Status | |--------|-------|--------| | Full test suite | 85 binaries, 0 failures | PASS | | New tests added | 1 (STORY-154-ALL-COVERAGEGAPS-TEST-001) | PASS | | Modified assertions | 3 tightened (STORY-152/154-WEAK-UNKNOWN-ASSERT-001) | PASS | | `cargo clippy --all-targets -- -D warnings` | Clean | PASS | | `cargo fmt --check` | Clean | PASS | ### New Test | Test | BC | Result | |------|----|--------| | `test_BC_2_12_023_all_with_coverage_gaps_combination()` | BC-2.12.023 Invariant 1, PC-1 | PASS | --- ## Holdout Evaluation N/A — evaluated at wave gate. No new behavioral contracts introduced. --- ## Adversarial Review These 4 findings originate from the F5 scoped-adversarial review pass (Phase F5). All are LOW severity / non-blocking. This PR resolves them before the F6 formal-hardening phase. | Finding ID | Category | Severity | Resolution | |------------|----------|----------|------------| | STORY-153-RUNANALYZE-DOC-STALE-001 | docs/comment accuracy | LOW | Doc-comment corrected | | STORY-154-LOOKUP-ARP-DEADCLAUSE-001 | dead code | LOW | ARP disjunct removed + proof comment | | STORY-154-ALL-COVERAGEGAPS-TEST-001 | test coverage gap | LOW | New integration test added | | STORY-152/154-WEAK-UNKNOWN-ASSERT-001 | test quality | LOW | 3 assertions tightened | --- ## Security Review **Verdict: APPROVE — no findings** ```mermaid graph LR Critical["Critical: 0"] High["High: 0"] Medium["Medium: 0"] Low["Low: 0"] style Critical fill:#90EE90 style High fill:#90EE90 style Medium fill:#90EE90 style Low fill:#90EE90 ``` OWASP Top 10 sweep: all N/A. Diff touches only internal match-guard logic and tests; no new input paths, no auth/authz, no dependencies, no network, no serialization. ARP dead-code removal is structurally sound — `find()` predicate eliminates `Transport::LinkLayer` entries before the removed disjunct could be evaluated. Test assertion tightening improves coverage properties; no security checks weakened. --- ## Risk Assessment ### Blast Radius - **Systems affected:** None (internal-only changes) - **User impact:** None (no behavior change; doc-comment + dead-code + test hardening) - **Data impact:** None - **Risk Level:** LOW ### Performance Impact No performance impact. The removed ARP disjunct was dead code in a match guard; its removal has no runtime effect. No new hot paths introduced. ### Feature Flags None. No feature flags used or required. --- ## Traceability | Finding | BC | Commit | File | Status | |---------|----|----|------|--------| | STORY-153-RUNANALYZE-DOC-STALE-001 | BC-2.05.010 | `7fbb57c` | `src/main.rs:193` | RESOLVED | | STORY-154-LOOKUP-ARP-DEADCLAUSE-001 | BC-2.12.023 | `0fdaa29` | `src/main.rs:1063` | RESOLVED | | STORY-154-ALL-COVERAGEGAPS-TEST-001 | BC-2.12.023 Inv.1/PC-1 | `abc048e` | `tests/integration_tests.rs` | RESOLVED | | STORY-152/154-WEAK-UNKNOWN-ASSERT-001 | BC-2.12.024 PC-4/EC-009/EC-010 | `f90dfb8` | `tests/integration_tests.rs` | RESOLVED | --- ## AI Pipeline Metadata <details> <summary><strong>Pipeline Details</strong></summary> ```yaml ai-generated: true pipeline-mode: feature factory-version: "1.0.0-rc.21" pipeline-stages: fix-pr-delivery: in-progress demo-recording: skipped (no behavior change) wave-integration-gate: skipped (fix PR — merges individually) fix-source: F5 scoped-adversarial review (E-21 feature-protocol-coverage) findings-resolved: 4 findings-severity: LOW models-used: pr-manager: claude-sonnet-4-6 security-reviewer: dispatched pr-reviewer: dispatched generated-at: "2026-07-04T00:00:00Z" ``` </details> --- ## Pre-Merge Checklist - [ ] All CI status checks passing - [x] No restricted files changed (only src/main.rs, tests/integration_tests.rs) - [x] No behavior change — docs/dead-code/test-hardening only - [x] All 4 F5 adversarial findings addressed - [ ] Security review: PENDING - [ ] PR reviewer: PENDING - [x] No demo recording required (no user-observable behavior change) - [x] No wave integration gate (fix PR — merges individually per autonomy level 4)
[STORY-152]
protocolsCLI Subcommand — Coverage Catalog Table + JSON OutputEpic: E-21 — feature-protocol-coverage
Wave: 68
Mode: feature
Convergence: CONVERGED after 3 adversarial passes (0 P0/CRITICAL/HIGH findings on d34a05f)
This PR delivers the
wirerust protocolsCLI subcommand (BC-2.12.022 + BC-2.18.001 + BC-2.18.002). It adds aCommands::Protocolsvariant with mutually-exclusive--all/--supported/--unsupportedfilter flags and a global--jsonpass-through, a terminal catalog table renderer (Name / Category / Transport / Port(s) / EtherType / Supported columns,[L2]indicator, port-102 collision footnote, LinkLayer note), and a structured JSON output path ({ "protocols": [...] }per BC-2.18.002 schema, stdout-only by design). The implementation consumes STORY-151'ssrc/protocols.rscatalog —all_protocols(),supported_protocols(),unsupported_protocols()— without touching theanalyzesubcommand.Diff: 5 files, +1268 lines.
tests/integration_tests.rsis a new file (1022 lines,mod story_152with 25 tests). The twocli_story_086_tests.rs(+2) andcli_story_096_tests.rs(+6) edits each add aCommands::Protocols {..} => panic!()arm to satisfy Rust's exhaustive match requirement for those files — these are blast-radius fallout from the new enum variant and are NOT functional changes to those test suites.Architecture Changes
graph TD CLI["src/cli.rs<br/>Commands::Protocols variant<br/>ProtocolFilter enum"] Main["src/main.rs<br/>dispatch arm<br/>run_protocols()"] Catalog["src/protocols.rs<br/>(STORY-151)<br/>all/supported/unsupported_protocols()"] Stdout["STDOUT<br/>terminal table or JSON"] CLI -->|"clap parse"| Main Main -->|"filter dispatch"| Catalog Catalog -->|"KnownProtocol slice"| Main Main -->|"render"| Stdout style CLI fill:#90EE90 style Main fill:#90EE90Architecture Decision Record — ADR-012
ADR-012: Protocol Coverage Catalog
Context: wirerust needed a static, queryable catalog of all ICS/IT protocols it knows about (both supported and unsupported) to answer "what gaps exist in my network coverage?" without running a live capture.
Decision 3: The
protocolssubcommand is a first-class top-levelCommandsvariant, not a sub-subcommand ofanalyze. Filter flags (--all/--supported/--unsupported) are mutually exclusive via clapconflicts_with. Default behavior (no flag) equals--all.Decision 7: JSON
"category"values are"ICS"or"IT"only — never"L2". The[L2]indicator is a transport-layer display annotation in the terminal renderer, not a category value.Rationale: Separation of concerns — catalog lookup is a pure, side-effect-free read with no pcap dependency. Keeping it as a separate
Commandsvariant avoids polluting theanalyzedispatch path and makes the help text clear to operators.Story Dependencies
graph LR S151["STORY-151<br/>✅ merged<br/>src/protocols.rs catalog"] S152["STORY-152<br/>🔶 this PR<br/>protocols subcommand"] S154["STORY-154<br/>⏳ pending<br/>depends on S152"] S151 --> S152 S152 --> S154 style S152 fill:#FFD700 style S151 fill:#90EE90 style S154 fill:#D3D3D3Dependency status: STORY-151 is merged (
src/protocols.rsis ondevelop). STORY-154 is pending and blocks on this PR.Spec Traceability
flowchart LR BC1["BC-2.12.022 v1.0<br/>CLI Dispatch + --json"] BC2["BC-2.18.001 v1.4<br/>Terminal Table Renderer"] BC3["BC-2.18.002 v1.1<br/>JSON Output Schema"] BC1 --> AC1["AC-152-001<br/>Commands::Protocols variant"] BC1 --> AC2["AC-152-002<br/>Dispatch to run_protocols()"] BC2 --> AC3["AC-152-003<br/>Terminal table rows/cols"] BC2 --> AC4["AC-152-004<br/>Port-102 footnote"] BC2 --> AC5["AC-152-005<br/>L2 note"] BC2 --> AC6["AC-152-006<br/>EtherType display"] BC3 --> AC7["AC-152-007<br/>JSON schema"] BC1 --> AC8["AC-152-008<br/>Exit 0; analyze unchanged"] AC1 --> T1["test_BC_2_12_022_protocols_subcommand_exit_0"] AC1 --> T2["test_BC_2_12_022_mutually_exclusive_flags_error"] AC2 --> T3["test_BC_2_12_022_protocols_json_flag"] AC3 --> T4["test_BC_2_18_001_all_row_count"] AC4 --> T5["test_BC_2_18_001_port102_footnote_names_all_four"] AC6 --> T6["test_BC_2_18_001_goose_ethertype_display<br/>0x88B8 (35000) — DF-CANONICAL-FRAME-HOLDOUT-001"] AC7 --> T7["test_BC_2_18_002_goose_json_canonical<br/>ethertype: 35000"] T1 --> S1["src/cli.rs + src/main.rs"] T4 --> S1 T6 --> S1 T7 --> S1Test Evidence
Coverage Summary
cargo test --all-targets)-D warnings)--check)Test Flow
graph LR UnitBc["BC-2.12.022 tests (5)<br/>CLI dispatch, flags, exit 0"] UnitBc2["BC-2.18.001 tests (13)<br/>Terminal table, EtherType, footnotes"] UnitBc3["BC-2.18.002 tests (7)<br/>JSON schema, canonical values"] Regression["Full regression<br/>cargo test --all-targets"] UnitBc -->|"PASS"| Green1["PASS"] UnitBc2 -->|"PASS"| Green2["PASS"] UnitBc3 -->|"PASS"| Green3["PASS"] Regression -->|"PASS"| Green4["PASS"] style Green1 fill:#90EE90 style Green2 fill:#90EE90 style Green3 fill:#90EE90 style Green4 fill:#90EE90New Tests (This PR)
New Tests — mod story_152 in tests/integration_tests.rs
test_BC_2_12_022_protocols_subcommand_exit_0test_BC_2_12_022_mutually_exclusive_flags_errortest_BC_2_12_022_protocols_supported_filtertest_BC_2_12_022_protocols_json_flagtest_BC_2_12_022_analyze_unaffectedtest_BC_2_18_001_all_row_counttest_BC_2_18_001_supported_filtertest_BC_2_18_001_port102_footnotetest_BC_2_18_001_port102_footnote_absent_supportedtest_BC_2_18_001_port102_footnote_names_all_fourtest_BC_2_18_001_l2_transport_indicatortest_BC_2_18_001_l2_note_presenttest_BC_2_18_001_goose_ethertype_displaytest_BC_2_18_001_powerlink_ethertype_displaytest_BC_2_18_001_arp_ethertype_dashtest_BC_2_18_002_json_schema_validtest_BC_2_18_002_l2_entries_no_portstest_BC_2_18_002_supported_flag_matches_functiontest_BC_2_18_002_goose_json_canonicaltest_BC_2_18_002_bacnet_json_canonicaltest_BC_2_18_002_modbus_json_canonicalDemo Evidence
Visual recordings (VHS 0.11.0 — GIF + WebM) are present in the worktree at
docs/demo-evidence/STORY-152/(untracked, not in PR diff by design — 5 per-AC recordings × 3 files each + evidence-report.md).AC-152-003-all-catalog(.gif/.webm)AC-152-004a-supported(.gif/.webm)AC-152-005-unsupported(.gif/.webm)0x88B8 (35000), POWERLINK0x88AB (34987), port-102 footnote naming all 4AC-152-007-json(.gif/.webm){"protocols":[...]}piped throughpython3 -m json.tool, valid structureAC-152-001-mutual-exclusion(.gif/.webm)--supported --unsupported, non-zero exitHoldout Evaluation
N/A — evaluated at wave gate. VP-041 regression/relevance reference (VP-041 harnesses anchored by STORY-151; this story consumes
supported_protocols()/unsupported_protocols()which VP-041 validates).Adversarial Review
Convergence: 3 consecutive clean passes on d34a05f — CONVERGED (DF-CONVERGENCE-BEFORE-MERGE-001 satisfied).
Canonical-frame holdout (DF-CANONICAL-FRAME-HOLDOUT-001): EtherType constants and port numbers independently verified:
0x88B8 = 35000(IEC 61850-8-1 §4; IEEE RA "IEC GOOSE")0x88AB = 34987(IEEE RA "ETHERNET Powerlink"; WiresharkETHERTYPE_EPL_V2)47808/0xBAC0(ASHRAE 135-2016 Annex J §J.2.1)502(IANA/Modbus App Protocol v1.1b3 §4.3.1)Deferred LOW finding: Global
--csv/--output-formatflags are no-ops when passed withprotocols(they are parsed but ignored byrun_protocols()). This is scoped-out per frozen BC-2.12.022 which models--jsonas aboolfor the protocols path; the PATH component of--json=<path>is also not used (stdout-only by design per BC-2.18.002 PC-1). Backlogged as STORY-152-GLOBAL-FLAG-NOOP-001.Pass 1 Findings and Resolutions
Finding STORY-152-Pass-1-F-2 (MEDIUM): AC-152-002 over-claim on --json=path routing
--json=<path>performs file-path routing "at the call site … follows the same pattern as the existing run_analyze() JSON path." This was an over-claim;protocolsemits to STDOUT only.protocols --jsonalways emits to STDOUT; PATH component of--json=<path>is NOT used; file-path routing is out of scope (unlikeanalyze).Finding STORY-152-Pass-1-F-3 (LOW): GOOSE derivation comment in implementation
src/main.rs(GOOSE supported-derivation comment)Security Review
graph LR Critical["Critical: 0"] High["High: 0"] Medium["Medium: 0"] Low["Low: 0 (INFO: 2)"] style Critical fill:#90EE90 style High fill:#90EE90 style Medium fill:#90EE90 style Low fill:#87CEEBVerdict: CLEAN — CRITICAL 0 | HIGH 0 | MEDIUM 0 | LOW 0 | INFO 2
Security Scan Details
CLI Argument Handling (CWE-88)
Not applicable. Three boolean flags (
--all/--supported/--unsupported) with clapconflicts_with_allmutual exclusion. No string-valued arguments, no positional arguments, no process execution insiderun_protocols.Output Rendering (CWE-134)
Not applicable. All
println!/format!format strings are compile-time literals. Data values are typed macro arguments.JSON Serialization (CWE-116)
Not applicable. Output via
serde_json::json!andserde_json::to_string_pretty. All values originate from&'static strcatalog names,u16ports,Option<u16>ethertypes, and booleans.serde_jsonescapes all string content unconditionally. No user input flows into JSON output.Path Traversal (CWE-22)
Not applicable.
run_protocolscontains zero file I/O. TheOption<PathBuf>inside--jsonis intentionally ignored —cli.json.is_some()is used as a pure boolean per frozen BC-2.18.002 PC-1.OS Command Injection (CWE-78)
Not applicable. No
Command::new,process::Command, or shell invocation in new code.Resource Consumption (CWE-400)
Not applicable.
KNOWN_PROTOCOLSis a compile-time&'static [KnownProtocol]constant with fixed length 30. Fully bounded at compile time.Supply Chain (OWASP A06)
No new dependencies. Pre-existing
serde_jsonandclapunchanged.INFO Observations (non-blocking, non-security)
INFO-001:
is_protocol_supportedcontains a hardcodedif p.name == "ARP"string literal atsrc/main.rs. This creates a maintenance coupling: if the catalog name ever changes, the branch silently stops matching. Not a security vulnerability; aconst ARP_NAMEshared reference would be cleaner. (Consistent with the same INFO observation from the PR reviewer's cosmetic finding.)INFO-002:
is_protocol_supportedcallssupported_protocols()per iteration inside the rendering loop — O(N²) at N=30 (900 comparisons). No practical impact; noted for completeness if catalog grows.Risk Assessment
Blast Radius
src/cli.rs(new enum variant — triggers exhaustive-match compiler errors in any file matching onCommands),src/main.rs(new dispatch arm andrun_protocols()function),tests/integration_tests.rs(new file)tests/cli_story_086_tests.rsandtests/cli_story_096_tests.rseach received aCommands::Protocols {..} => panic!()arm. These are compile-required changes with no behavioral effect on those test suites — reviewers should NOT flag them as stray modifications.protocolssubcommand.analyzesubcommand behavior is unchanged (AC-152-008,test_BC_2_12_022_analyze_unaffected).Performance Impact
analyzepathRollback Instructions
Immediate rollback:
After rollback,
wirerust protocolswill returnerror: unrecognized subcommand 'protocols'.wirerust analyzeis unaffected.Feature Flags
None. The
protocolssubcommand is always available once this PR merges.Traceability
test_BC_2_12_022_protocols_subcommand_exit_0test_BC_2_12_022_mutually_exclusive_flags_errortest_BC_2_12_022_protocols_supported_filtertest_BC_2_12_022_protocols_json_flagtest_BC_2_12_022_analyze_unaffectedtest_BC_2_18_001_all_row_counttest_BC_2_18_001_port102_footnote_names_all_fourtest_BC_2_18_001_l2_note_presenttest_BC_2_18_001_goose_ethertype_displaytest_BC_2_18_001_powerlink_ethertype_displaytest_BC_2_18_002_json_schema_validtest_BC_2_18_002_goose_json_canonicaltest_BC_2_18_002_bacnet_json_canonicaltest_BC_2_18_002_modbus_json_canonicalFull VSDD Contract Chain
AI Pipeline Metadata
Pipeline Details
Pre-Merge Checklist
cargo test --all-targetsregression cleancargo clippy --all-targets -- -D warningscleancargo fmt --checkclean