From ab5d1b3e83329bdebe9352081a7478fc08b690ce Mon Sep 17 00:00:00 2001 From: UncleSp1d3r Date: Sat, 4 Jul 2026 22:23:49 -0400 Subject: [PATCH 1/5] feat(cli): add --imports/--exports/--symbols convenience flags Add three boolean shorthands for the most common reverse-engineering tag filters: --imports (import), --exports (export), --symbols (demangled symbols). Each is behaviorally equivalent to the corresponding --only-tags value, conflicts with --only-tags and --raw at parse time, and composes with --no-tags via a resolved include set (Cli::resolved_include_tags). The runtime include/exclude overlap check and its error message are generalized to cover the resolved include set, not just --only-tags. Closes #208. Signed-off-by: UncleSp1d3r --- AGENTS.md | 5 +- ...-04-001-feat-cli-convenience-flags-plan.md | 182 +++++++++++++++++ src/main.rs | 50 ++++- tests/integration_cli.rs | 189 +++++++++++++++++- tests/integration_cli_coverage.rs | 2 +- tests/integration_flow8_errors.rs | 2 +- 6 files changed, 419 insertions(+), 11 deletions(-) create mode 100644 docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md diff --git a/AGENTS.md b/AGENTS.md index acc35054..c5759249 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,10 @@ Use idiomatic `clap` derive API patterns. Push validation into clap wherever pos | `--json` | `-j` | bool | Conflicts with `--yara` | | `--yara` | | bool | Conflicts with `--json` | | `--only-tags` | | `Vec` | Repeatable, `value_parser = Tag::from_str` | -| `--no-tags` | | `Vec` | Repeatable, runtime overlap check with `--only-tags` | +| `--no-tags` | | `Vec` | Repeatable, runtime overlap check against the resolved include set (`--only-tags` plus the convenience flags) | +| `--imports` | | bool | Shorthand for `--only-tags import`; conflicts with `--only-tags`/`--raw`, compatible with `--no-tags` | +| `--exports` | | bool | Shorthand for `--only-tags export`; conflicts with `--only-tags`/`--raw`, compatible with `--no-tags` | +| `--symbols` | | bool | Shorthand for `--only-tags demangled` (DemangledSymbol only); conflicts with `--only-tags`/`--raw`, compatible with `--no-tags` | | `--min-len` | `-m` | `Option` | Custom parser enforces >= 1 | | `--top` | `-t` | `Option` | Custom parser enforces >= 1 | | `--enc` | | `Option` | utf8, utf16, utf16le, utf16be filter by stored encoding; `ascii` is content-based (matches pure-ASCII text that is not UTF-16, since extraction labels ASCII content as UTF-8) | diff --git a/docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md b/docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md new file mode 100644 index 00000000..11cce7b4 --- /dev/null +++ b/docs/plans/2026-07-04-001-feat-cli-convenience-flags-plan.md @@ -0,0 +1,182 @@ +--- +title: CLI Convenience Flags (--imports / --exports / --symbols) - Plan +type: feat +date: 2026-07-04 +topic: cli-convenience-flags +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-brainstorm +execution: code +--- + +# CLI Convenience Flags (--imports / --exports / --symbols) - Plan + +## Goal Capsule + +- **Objective:** Add three convenience CLI flags that shorthand the most common reverse-engineering tag filters, so users don't need to know exact `Tag` spellings. +- **Product authority:** Issue [#208](https://github.com/EvilBit-Labs/Stringy/issues/208) (follow-up to #40 Feature 3); scope confirmed via brainstorm. +- **Execution profile:** CLI-only change in `src/main.rs`, test-first per the repo TDD policy. No pipeline, extraction, classification, or `Tag` enum changes. +- **Open blockers:** None. Design decisions resolved. + +--- + +## Product Contract + +### Summary + +Add `--imports`, `--exports`, and `--symbols` as boolean shorthands for `--only-tags import` / `export` / `demangled`. The set is deliberately closed and reverse-engineering-focused; each flag's help text points to `--only-tags` for the full tag vocabulary. + +### Problem Frame + +Imports, exports, and demangled symbols are already extracted, tagged, classified, and ranked. They are filterable today only through `--only-tags `, which forces the user to know the canonical tag spelling. The mismatch bites hardest for symbols: the tag is `demangled`, but analysts reach for the word "symbols." These three queries ("what does this binary import / export / expose") are the highest-frequency RE tasks, so the vocabulary friction lands on the most-used paths. + +### Key Decisions + +- **Closed, RE-focused set of three.** These three earn flags because their tag names don't match user vocabulary and they are the top-frequency RE queries -- not because "tags deserve flags." The other ~22 tags (`url`, `ipv4`, `crypto`, `network`, ...) stay on `--only-tags`, which already reads self-evidently. This avoids `--help` bloat and per-tag conflict-matrix churn. +- **`--symbols` maps to `DemangledSymbol` only.** Keeping it distinct from `--imports`/`--exports` makes all three orthogonal, so `--imports --exports --symbols` yields a clean union rather than overlapping sets. +- **Conflict with `--only-tags` and `--raw`; compatible with `--no-tags`.** Both convenience flags and `--only-tags` populate the include set, so mixing them is ambiguous and forbidden at parse time. `--raw` skips classification and clears tags, so any tag filter is meaningless there. `--no-tags` (exclusion) composes cleanly, reusing the existing include/exclude overlap check. +- **Help-text pointer signals the closed-set intent.** Each flag notes it is a shorthand for a common analysis tag and points to `--only-tags`, so the absence of `--urls`/`--crypto` reads as deliberate rather than an oversight. + +### Requirements + +**Behavior** + +- R1. `--imports`, `--exports`, and `--symbols` are boolean flags that filter output to strings tagged `Import`, `Export`, and `DemangledSymbol` respectively -- behaviorally equivalent to the corresponding `--only-tags` value. +- R2. `--symbols` maps to `DemangledSymbol` only; it does not also include `Import` or `Export`. +- R3. The three flags combine with each other as OR (union), matching repeated `--only-tags`. + +**Conflicts and composition** + +- R4. The three flags conflict with `--only-tags` and with `--raw`, enforced by clap at parse time. +- R5. The three flags are compatible with `--no-tags`; the existing runtime overlap validation applies to the resolved include set, so a contradiction such as `--imports --no-tags import` still errors. + +**Surface** + +- R6. Each flag's help text identifies it as a shorthand for a common analysis tag and points to `--only-tags` for the full tag set. +- R7. Flags are long-only, with no short aliases -- consistent with `--enc`/`--yara`/`--raw`. + +### Acceptance Examples + +- AE1. **Covers R3.** Given a binary, when the user runs `--imports --exports`, then output includes both import- and export-tagged strings (union), exit success. +- AE2. **Covers R4.** When the user runs `--imports --only-tags url`, then clap rejects with a conflict error (non-zero exit). +- AE3. **Covers R4.** When the user runs `--symbols --raw`, then clap rejects with a conflict error. +- AE4. **Covers R5.** When the user runs `--imports --no-tags import`, then the run fails the overlap validation with a clear message. +- AE5. **Covers R5.** When the user runs `--imports --no-tags version`, then the run succeeds (no contradiction). + +### Scope Boundaries + +- Convenience flags for any other tag (`url`, `ipv4`, `crypto`, `network`, `guid`, ...) -- out; use `--only-tags`. +- Short aliases for the new flags -- out (R7). +- A tag-alias mechanism that makes `--only-tags symbols` resolve to `demangled` -- out; the decision is top-level flags, not alias resolution. +- Changes to extraction, classification, ranking, or the `Tag` enum -- out; this is CLI-only ergonomics. + +--- + +## Planning Contract + +**Product Contract preservation:** Product Contract unchanged. This enrichment adds the Planning Contract, Implementation Units, Verification Contract, and Definition of Done only; all R/AE IDs and product-scope text are preserved verbatim. + +### Key Technical Decisions + +- KTD1. **Resolve the include set through a single `Cli::resolved_include_tags()` helper.** The helper returns `only_tags` plus any convenience flags mapped to their `Tag` variant. `run()` computes it once and reuses it for both the overlap check and the filter build, so there is one authoritative include set. Keeping the mapping in the helper (rather than inline branches in `run()`) holds the added lines in `run()` to a single call plus reuse -- `run()` is already ~87 lines, so avoid inflating it further. Convenience flags conflict with `--only-tags` at parse time, so in practice only one source populates the set, but merging is harmless and future-proof. Follows the existing `From`-style mapping and `FilterConfig` builder conventions in `src/main.rs`. +- KTD2. **Push all mutual exclusion into clap `conflicts_with_all`.** Each new field carries `conflicts_with_all = ["only_tags", "raw"]`, mirroring the existing `--raw` field. clap conflicts are symmetric, so `--raw` automatically conflicts with the three new flags; adding the three names to `--raw`'s existing `conflicts_with_all` list is optional readability polish, not a correctness requirement. Keep the include-vs-exclude (`--no-tags`) check in `run()` as runtime validation, since clap cannot express "same tag on both sides." +- KTD3. **Generalize the overlap error message so it is accurate for both include sources.** The current message names "`--only-tags` and `--no-tags`", which would be misleading for an `--imports --no-tags import` contradiction. Reword it to describe an include/exclude contradiction without hard-coding `--only-tags` (e.g., name the conflicting tag and that it is both included and excluded via `--no-tags`). This is a deliberate, in-scope co-change: the existing `cli_overlapping_tags_rejected` test asserts the old substring and must be updated to a stable substring that holds for both paths. + +### Assumptions + +- A fixture with import-tagged strings exists for the behavioral-equivalence test (`tests/fixtures/test_binary_pe.exe` is the expected source). If the chosen fixture yields zero import-tagged strings, the equivalence assertion still holds (both sides empty) but proves less; U1 notes selecting a fixture with known imports and treating an empty-both result as a weak pass. +- Clap conflict errors exit with code 2 (consistent with `cli_json_and_yara_conflict`), so conflict tests may assert `.code(2)`. + +### Sequencing + +U1 (tests, RED) is authored first, then U2 (implementation, GREEN) makes them pass, then U3 (docs) closes the AGENTS.md co-change. U3 depends only on the final flag surface from U2. + +--- + +## Implementation Units + +### U1. Integration tests for the three convenience flags + +- **Goal:** Add the failing integration tests that pin down flag behavior, conflicts, composition, help surface, and behavioral equivalence before any implementation exists. +- **Requirements:** R1, R2, R3, R4, R5, R6. +- **Dependencies:** none. +- **Files:** `tests/integration_cli.rs` +- **Approach:** Mirror the existing `assert_cmd` patterns (`cli_only_tags_filter`, `cli_json_and_yara_conflict`, `cli_raw_conflicts_with_yara`, `cli_overlapping_tags_rejected`, `cli_help_lists_all_canonical_tags`). Tests invoke the built binary and do not reference internal symbols, so they compile before U2 lands and fail at runtime until it does. Update `cli_overlapping_tags_rejected` to assert a stable substring compatible with the generalized message from KTD3 (assert on the tag name plus an include/exclude phrase, not the literal "`--only-tags and --no-tags`"). +- **Execution note:** Author this unit first (RED). Run it and confirm each new test fails for the right reason (unknown flag / missing behavior) before starting U2. +- **Patterns to follow:** `tests/integration_cli.rs` existing conflict and success tests; use `tests/fixtures/test_binary_elf` for success/conflict cases and `tests/fixtures/test_binary_pe.exe` for import-bearing equivalence. +- **Test scenarios:** + - `--imports` on an ELF fixture exits success (happy path). + - `--exports` on an ELF fixture exits success (happy path). + - `--symbols` on an ELF fixture exits success (happy path). + - Covers AE1 / R3. `--imports --exports --json` exits success and every emitted JSON row carries `Import` or `Export` (union), asserting no row lacks both -- and that the combined line count is greater than or equal to `--imports` alone (OR, not AND). + - Covers AE2 / R4. `--imports --only-tags url` fails at parse time with exit code 2 and stderr containing "cannot be used with". + - Covers AE3 / R4. `--symbols --raw` fails at parse time with exit code 2 and stderr containing "cannot be used with". + - Covers AE4 / R5. `--imports --no-tags import` fails the runtime overlap check with a clear message naming the `import` tag. + - Covers AE5 / R5. `--imports --no-tags version` exits success. + - Covers R2. `--symbols --json` output rows all carry the `demangled` tag and none is emitted solely for an `Import`/`Export` tag (symbols maps to `DemangledSymbol` only). + - Covers R6. `--help` output lists `--imports`, `--exports`, and `--symbols` and each references `--only-tags`. + - Behavioral equivalence (the confirmed call-out): on `tests/fixtures/test_binary_pe.exe`, `--imports --json` output equals `--only-tags import --json` output. Select a fixture with known imports; treat an empty-both result as a weak pass and note it. + - Update: `cli_overlapping_tags_rejected` still fails `--only-tags url --no-tags url` but now asserts the generalized-message substring. +- **Verification:** `cargo nextest run --test integration_cli` shows the new tests present and failing (before U2), then passing (after U2). `assert_cmd` runs non-TTY -- do not assert TTY table formatting here. + +### U2. Implement the flags and include-tag resolution in `src/main.rs` + +- **Goal:** Add the three boolean clap fields, the `resolved_include_tags()` helper, and rewire `run()` so U1 passes. +- **Requirements:** R1, R2, R3, R4, R5, R6, R7. +- **Dependencies:** U1. +- **Files:** `src/main.rs` +- **Approach:** + - Add three `bool` fields to `struct Cli`, each `#[arg(long, conflicts_with_all = ["only_tags", "raw"])]`, with doc comments that name the shorthand and point to `--only-tags` (satisfies R6; long-only satisfies R7). Optionally add `"imports"`, `"exports"`, `"symbols"` to the `--raw` field's existing `conflicts_with_all` for help readability (symmetric, not required for correctness). + - Add `Cli::resolved_include_tags(&self) -> Vec`: clone `only_tags`, then push `Tag::Import` / `Tag::Export` / `Tag::DemangledSymbol` for each set flag (KTD1). `--symbols` pushes only `DemangledSymbol` (R2). Multiple flags append, giving OR/union (R3). + - In `run()`: compute the include set once (`let include_tags = cli.resolved_include_tags();`) near the top. Replace the overlap-check source (currently `cli.only_tags.iter()`, main.rs:155-159) with `include_tags`, and generalize the error message per KTD3. Replace the filter-build guard (currently `if !cli.only_tags.is_empty()`, main.rs:208-210) with the resolved set. +- **Execution note:** Implement to turn U1 green (GREEN). `run()` is already ~87 lines, so keep the added footprint to the single `resolved_include_tags()` call plus its reuse; do not inline per-flag branches into `run()`. If the include-resolution or validation grows, lift it into a helper rather than growing `run()`. +- **Patterns to follow:** existing `--only-tags`/`--no-tags`/`--raw` clap attributes and the `FilterConfig` builder chain in `run()`; `From for EncodingFilter` as the mapping-style precedent. +- **Test scenarios:** none new -- U1 is the authoritative behavioral spec for this unit. (Any pure-refactor helper extraction is covered transitively by U1.) +- **Verification:** `just check` (fmt + clippy `-D warnings` + nextest) is clean; all U1 tests pass; no new clippy warnings; `#![forbid(unsafe_code)]` preserved. + +### U3. Update the AGENTS.md CLI-flag table + +- **Goal:** Keep the documented flag surface in sync with the implemented flags (repo co-change policy). +- **Requirements:** supports R6 discoverability; satisfies the AGENTS.md flag-table item in the Definition of Done. +- **Dependencies:** U2. +- **Files:** `AGENTS.md` +- **Approach:** Add `--imports`, `--exports`, `--symbols` rows to the "Current CLI Flags (main.rs)" table: long-only, `bool`, note each is a shorthand for the corresponding `--only-tags` value and conflicts with `--only-tags`/`--raw`, compatible with `--no-tags`. Match the existing table's column shape and tone. Also update the existing `--no-tags` row -- its note currently says the runtime overlap check is "with `--only-tags`", which KTD3 generalizes; reword it to say the overlap check applies to the resolved include set (`--only-tags` plus the convenience flags). +- **Test expectation:** none -- documentation-only change with no behavioral surface. +- **Verification:** table renders correctly and the three rows accurately describe the shipped flags; `just format` leaves the Markdown clean. + +--- + +## Verification Contract + +| Gate | Command | Applies to | Done signal | +| ------------------------ | ------------------------------------------------ | ---------- | --------------------------------------------------------- | +| Unit + integration tests | `cargo nextest run` (via `just test`) | U1, U2 | All tests pass, including the new `integration_cli` cases | +| Format | `cargo fmt --check` (via `just check`) | U2, U3 | No diff | +| Lint | `cargo clippy -- -D warnings` (via `just check`) | U2 | Zero warnings | +| Full pre-commit | `just check` | U1-U3 | Exit 0 | +| Full CI locally | `just ci-check` | U1-U3 | Exit 0 (final gate before declaring done) | + +Behavioral equivalence gate: `--imports` output equals `--only-tags import` output on a PE fixture with known imports (asserted in U1). + +--- + +## Definition of Done + +- [ ] `--imports`, `--exports`, `--symbols` implemented as long-only clap `bool` flags (R1, R7). +- [ ] `--symbols` maps to `DemangledSymbol` only; `--imports --exports --symbols` yields the union (R2, R3). +- [ ] Conflicts with `--only-tags` and `--raw` enforced by clap at parse time (R4). +- [ ] Compatible with `--no-tags`; the resolved include set drives the runtime overlap check so `--imports --no-tags import` errors and `--imports --no-tags version` succeeds (R5). +- [ ] Each flag's help text points to `--only-tags` (R6). +- [ ] `tests/integration_cli.rs` covers success, each conflict, combine-together, `--no-tags` contradiction, `--help` listing, and behavioral equivalence; `cli_overlapping_tags_rejected` updated for the generalized message. +- [ ] `AGENTS.md` "Current CLI Flags" table gains the three rows, and the existing `--no-tags` row reflects the generalized (resolved-include-set) overlap check. +- [ ] `just ci-check` passes. + +--- + +## Sources / Research + +- `src/main.rs` -- `Cli` struct (fields at lines 89-151); existing `--only-tags`/`--no-tags`/`--raw` clap patterns; `run()` overlap check (lines 155-167) and filter build (lines 208-213) are the two rewire points; `From` mapping precedent. +- `src/types/mod.rs` -- `Tag::Import`, `Tag::Export`, `Tag::DemangledSymbol` variants and `Tag::from_str`. +- `tests/integration_cli.rs` -- `cli_only_tags_filter`, `cli_json_and_yara_conflict`, `cli_raw_conflicts_with_yara`, `cli_overlapping_tags_rejected`, `cli_help_lists_all_canonical_tags` are the patterns to mirror. +- `AGENTS.md` -- "Current CLI Flags" table and the GOTCHAS note that CLI flag changes must co-change tests and the table. +- `GOTCHAS.md` (CLI section) -- clap conflict exit codes are 2 for config/validation; `--raw` early-exit semantics; help-text/`from_str` sync test. diff --git a/src/main.rs b/src/main.rs index 4ec4ba41..c2bd1067 100644 --- a/src/main.rs +++ b/src/main.rs @@ -125,6 +125,18 @@ struct Cli { )] no_tags: Vec, + /// Shorthand for --only-tags import (see --only-tags for the full tag set) + #[arg(long, conflicts_with_all = ["only_tags", "raw"])] + imports: bool, + + /// Shorthand for --only-tags export (see --only-tags for the full tag set) + #[arg(long, conflicts_with_all = ["only_tags", "raw"])] + exports: bool, + + /// Shorthand for --only-tags demangled (see --only-tags for the full tag set) + #[arg(long, conflicts_with_all = ["only_tags", "raw"])] + symbols: bool, + /// Minimum string length in bytes (must be >= 1) #[arg(short = 'm', long = "min-len", value_name = "N", value_parser = parse_positive_usize)] min_len: Option, @@ -150,18 +162,42 @@ struct Cli { debug: bool, } +impl Cli { + /// Effective include-tag set: explicit `--only-tags` plus the convenience + /// flags (`--imports` / `--exports` / `--symbols`). The convenience flags + /// conflict with `--only-tags` at parse time, so in practice only one source + /// populates the set, but merging is harmless and keeps a single + /// authoritative include list. + fn resolved_include_tags(&self) -> Vec { + let mut tags = self.only_tags.clone(); + if self.imports { + tags.push(Tag::Import); + } + if self.exports { + tags.push(Tag::Export); + } + if self.symbols { + tags.push(Tag::DemangledSymbol); + } + tags + } +} + fn run(cli: &Cli) -> Result<(), StringyError> { - // Runtime validation: tag overlap between --only-tags and --no-tags - let overlap: Vec<&Tag> = cli - .only_tags + // Resolve the effective include-tag set once: --only-tags plus the + // convenience flags (--imports / --exports / --symbols). + let include_tags = cli.resolved_include_tags(); + + // Runtime validation: tag overlap between the resolved include set and --no-tags + let overlap: Vec<&Tag> = include_tags .iter() .filter(|t| cli.no_tags.contains(t)) .collect(); if !overlap.is_empty() { let tag_names: Vec = overlap.iter().map(|t| format!("{t:?}")).collect(); return Err(StringyError::ValidationError(format!( - "conflicting tag filters: {} appear in both --only-tags and --no-tags\n\ - Remove these tags from one of the filter lists to continue.", + "conflicting tag filters: {} are both included and excluded (--no-tags)\n\ + Remove these tags from the include filter or from --no-tags to continue.", tag_names.join(", ") ))); } @@ -205,8 +241,8 @@ fn run(cli: &Cli) -> Result<(), StringyError> { if let Some(enc) = cli.enc { filter_config = filter_config.with_encoding(enc.into()); } - if !cli.only_tags.is_empty() { - filter_config = filter_config.with_include_tags(cli.only_tags.clone()); + if !include_tags.is_empty() { + filter_config = filter_config.with_include_tags(include_tags.clone()); } if !cli.no_tags.is_empty() { filter_config = filter_config.with_exclude_tags(cli.no_tags.clone()); diff --git a/tests/integration_cli.rs b/tests/integration_cli.rs index 28f11c5e..6a3d0677 100644 --- a/tests/integration_cli.rs +++ b/tests/integration_cli.rs @@ -155,7 +155,7 @@ fn cli_overlapping_tags_rejected() { ]) .assert() .failure() - .stderr(predicate::str::contains("--only-tags and --no-tags")); + .stderr(predicate::str::contains("conflicting tag filters")); } #[test] @@ -320,3 +320,190 @@ fn cli_only_tags_filter_excludes_untagged() { ); } } + +// -- Convenience flags: --imports / --exports / --symbols (issue #208) -- + +/// Collect non-empty stdout lines, sorted, so equivalence checks are robust to +/// any run-to-run ordering nondeterminism in extraction or score tie-breaking. +fn sorted_stdout_lines(bytes: &[u8]) -> Vec { + let text = String::from_utf8_lossy(bytes); + let mut lines: Vec = text + .lines() + .filter(|l| !l.is_empty()) + .map(|l| l.to_string()) + .collect(); + lines.sort(); + lines +} + +#[test] +fn cli_imports_flag_succeeds() { + stringy() + .args(["tests/fixtures/test_binary_elf", "--imports"]) + .assert() + .success(); +} + +#[test] +fn cli_exports_flag_succeeds() { + stringy() + .args(["tests/fixtures/test_binary_elf", "--exports"]) + .assert() + .success(); +} + +#[test] +fn cli_symbols_flag_succeeds() { + stringy() + .args(["tests/fixtures/test_binary_elf", "--symbols"]) + .assert() + .success(); +} + +#[test] +fn cli_imports_equivalent_to_only_tags_import() { + // R1: --imports is behaviorally equivalent to --only-tags import. + let flag = stringy() + .args(["tests/fixtures/test_binary_pe.exe", "--imports", "--json"]) + .output() + .expect("should succeed"); + let tag = stringy() + .args([ + "tests/fixtures/test_binary_pe.exe", + "--only-tags", + "import", + "--json", + ]) + .output() + .expect("should succeed"); + assert!(flag.status.success() && tag.status.success()); + assert_eq!( + sorted_stdout_lines(&flag.stdout), + sorted_stdout_lines(&tag.stdout), + "--imports should produce the same rows as --only-tags import" + ); +} + +#[test] +fn cli_symbols_equivalent_to_only_tags_demangled() { + // R2: --symbols maps to DemangledSymbol only; equivalent to --only-tags demangled. + let flag = stringy() + .args(["tests/fixtures/test_binary_pe.exe", "--symbols", "--json"]) + .output() + .expect("should succeed"); + let tag = stringy() + .args([ + "tests/fixtures/test_binary_pe.exe", + "--only-tags", + "demangled", + "--json", + ]) + .output() + .expect("should succeed"); + assert!(flag.status.success() && tag.status.success()); + assert_eq!( + sorted_stdout_lines(&flag.stdout), + sorted_stdout_lines(&tag.stdout), + "--symbols should produce the same rows as --only-tags demangled" + ); +} + +#[test] +fn cli_convenience_flags_combine_as_union() { + // AE1 / R3: --imports --exports equals repeated --only-tags import export (union). + let flags = stringy() + .args([ + "tests/fixtures/test_binary_pe.exe", + "--imports", + "--exports", + "--json", + ]) + .output() + .expect("should succeed"); + let tags = stringy() + .args([ + "tests/fixtures/test_binary_pe.exe", + "--only-tags", + "import", + "--only-tags", + "export", + "--json", + ]) + .output() + .expect("should succeed"); + assert!(flags.status.success() && tags.status.success()); + assert_eq!( + sorted_stdout_lines(&flags.stdout), + sorted_stdout_lines(&tags.stdout), + "--imports --exports should equal repeated --only-tags import export" + ); +} + +#[test] +fn cli_imports_conflicts_with_only_tags() { + // AE2 / R4: convenience flag conflicts with --only-tags at parse time. + stringy() + .args([ + "tests/fixtures/test_binary_elf", + "--imports", + "--only-tags", + "url", + ]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); +} + +#[test] +fn cli_symbols_conflicts_with_raw() { + // AE3 / R4: convenience flag conflicts with --raw at parse time. + stringy() + .args(["tests/fixtures/test_binary_elf", "--symbols", "--raw"]) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); +} + +#[test] +fn cli_imports_no_tags_contradiction_rejected() { + // AE4 / R5: resolved include set overlapping --no-tags triggers runtime validation. + stringy() + .args([ + "tests/fixtures/test_binary_elf", + "--imports", + "--no-tags", + "import", + ]) + .assert() + .failure() + .stderr(predicate::str::contains("conflicting tag filters")); +} + +#[test] +fn cli_imports_no_tags_compatible() { + // AE5 / R5: no contradiction between --imports and --no-tags version. + stringy() + .args([ + "tests/fixtures/test_binary_elf", + "--imports", + "--no-tags", + "version", + ]) + .assert() + .success(); +} + +#[test] +fn cli_help_lists_convenience_flags() { + // R6: help lists the three flags and each points to --only-tags. + stringy() + .arg("--help") + .assert() + .success() + .stdout(predicate::str::contains("--imports")) + .stdout(predicate::str::contains("--exports")) + .stdout(predicate::str::contains("--symbols")) + .stdout(predicate::str::contains("Shorthand for --only-tags")); +} diff --git a/tests/integration_cli_coverage.rs b/tests/integration_cli_coverage.rs index bf7caa16..6a69cf8b 100644 --- a/tests/integration_cli_coverage.rs +++ b/tests/integration_cli_coverage.rs @@ -231,7 +231,7 @@ fn exit_code_2_validation_error() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("--only-tags and --no-tags")); + .stderr(predicate::str::contains("conflicting tag filters")); } #[test] diff --git a/tests/integration_flow8_errors.rs b/tests/integration_flow8_errors.rs index fcf080bb..3163bd63 100644 --- a/tests/integration_flow8_errors.rs +++ b/tests/integration_flow8_errors.rs @@ -124,5 +124,5 @@ fn flow8_tag_overlap_exit_2() { .assert() .failure() .code(2) - .stderr(predicate::str::contains("--only-tags and --no-tags")); + .stderr(predicate::str::contains("conflicting tag filters")); } From 5b2b1448104d1f4b25b99d14ac88fe3a7be6fc88 Mon Sep 17 00:00:00 2001 From: UncleSp1d3r Date: Sat, 4 Jul 2026 22:42:51 -0400 Subject: [PATCH 2/5] refactor(cli): resolve include tags without an extra clone Move the resolved include-tag vec into with_include_tags instead of cloning it (the earlier overlap borrow is dead under NLL), and drop the inline comment that restated the helper's doc comment. No behavior change. Signed-off-by: UncleSp1d3r --- src/main.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index c2bd1067..1d3c285e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -184,8 +184,6 @@ impl Cli { } fn run(cli: &Cli) -> Result<(), StringyError> { - // Resolve the effective include-tag set once: --only-tags plus the - // convenience flags (--imports / --exports / --symbols). let include_tags = cli.resolved_include_tags(); // Runtime validation: tag overlap between the resolved include set and --no-tags @@ -242,7 +240,7 @@ fn run(cli: &Cli) -> Result<(), StringyError> { filter_config = filter_config.with_encoding(enc.into()); } if !include_tags.is_empty() { - filter_config = filter_config.with_include_tags(include_tags.clone()); + filter_config = filter_config.with_include_tags(include_tags); } if !cli.no_tags.is_empty() { filter_config = filter_config.with_exclude_tags(cli.no_tags.clone()); From e61d43a47400bc182047a7c1674fc2cfd5123108 Mon Sep 17 00:00:00 2001 From: UncleSp1d3r Date: Sat, 4 Jul 2026 22:43:18 -0400 Subject: [PATCH 3/5] test(cli): strengthen and split convenience-flag coverage Move the --imports/--exports/--symbols tests into a dedicated integration_cli_convenience_flags.rs (keeps integration_cli.rs under the 500-line guideline). Run equivalence tests against test_binary_elf, which has non-zero import (81) and export (9) rows -- the previous PE fixture had zero exports, making the export/union assertions vacuous. Guard --symbols against aliasing --imports/--exports (no fixture has demangled rows), assert non-empty output where tags exist, and cover all six flag/conflict pairs. Sync the mdBook CLI reference with the three new flags. Signed-off-by: UncleSp1d3r --- docs/src/cli.md | 17 +- tests/integration_cli.rs | 187 --------------------- tests/integration_cli_convenience_flags.rs | 153 +++++++++++++++++ 3 files changed, 163 insertions(+), 194 deletions(-) create mode 100644 tests/integration_cli_convenience_flags.rs diff --git a/docs/src/cli.md b/docs/src/cli.md index 62080a72..c20c0c3d 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -21,13 +21,16 @@ stringy [OPTIONS] - # read from stdin ### Filtering -| Option | Description | Default | -| ----------------- | ------------------------------------------------------------------ | ------- | -| `--min-len N` | Minimum string length (must be >= 1) | 4 | -| `--top N` | Limit to top N strings by score (applied after all filters) | - | -| `--enc ENCODING` | Filter by encoding: `ascii`, `utf8`, `utf16`, `utf16le`, `utf16be` | all | -| `--only-tags TAG` | Include strings with any of these tags (OR); repeatable | all | -| `--no-tags TAG` | Exclude strings with any of these tags; repeatable | none | +| Option | Description | Default | +| ----------------- | --------------------------------------------------------------------------- | ------- | +| `--min-len N` | Minimum string length (must be >= 1) | 4 | +| `--top N` | Limit to top N strings by score (applied after all filters) | - | +| `--enc ENCODING` | Filter by encoding: `ascii`, `utf8`, `utf16`, `utf16le`, `utf16be` | all | +| `--only-tags TAG` | Include strings with any of these tags (OR); repeatable | all | +| `--no-tags TAG` | Exclude strings with any of these tags; repeatable | none | +| `--imports` | Shorthand for `--only-tags import`; conflicts with `--only-tags`/`--raw` | all | +| `--exports` | Shorthand for `--only-tags export`; conflicts with `--only-tags`/`--raw` | all | +| `--symbols` | Shorthand for `--only-tags demangled`; conflicts with `--only-tags`/`--raw` | all | ### Mode Flags diff --git a/tests/integration_cli.rs b/tests/integration_cli.rs index 6a3d0677..3233d91c 100644 --- a/tests/integration_cli.rs +++ b/tests/integration_cli.rs @@ -320,190 +320,3 @@ fn cli_only_tags_filter_excludes_untagged() { ); } } - -// -- Convenience flags: --imports / --exports / --symbols (issue #208) -- - -/// Collect non-empty stdout lines, sorted, so equivalence checks are robust to -/// any run-to-run ordering nondeterminism in extraction or score tie-breaking. -fn sorted_stdout_lines(bytes: &[u8]) -> Vec { - let text = String::from_utf8_lossy(bytes); - let mut lines: Vec = text - .lines() - .filter(|l| !l.is_empty()) - .map(|l| l.to_string()) - .collect(); - lines.sort(); - lines -} - -#[test] -fn cli_imports_flag_succeeds() { - stringy() - .args(["tests/fixtures/test_binary_elf", "--imports"]) - .assert() - .success(); -} - -#[test] -fn cli_exports_flag_succeeds() { - stringy() - .args(["tests/fixtures/test_binary_elf", "--exports"]) - .assert() - .success(); -} - -#[test] -fn cli_symbols_flag_succeeds() { - stringy() - .args(["tests/fixtures/test_binary_elf", "--symbols"]) - .assert() - .success(); -} - -#[test] -fn cli_imports_equivalent_to_only_tags_import() { - // R1: --imports is behaviorally equivalent to --only-tags import. - let flag = stringy() - .args(["tests/fixtures/test_binary_pe.exe", "--imports", "--json"]) - .output() - .expect("should succeed"); - let tag = stringy() - .args([ - "tests/fixtures/test_binary_pe.exe", - "--only-tags", - "import", - "--json", - ]) - .output() - .expect("should succeed"); - assert!(flag.status.success() && tag.status.success()); - assert_eq!( - sorted_stdout_lines(&flag.stdout), - sorted_stdout_lines(&tag.stdout), - "--imports should produce the same rows as --only-tags import" - ); -} - -#[test] -fn cli_symbols_equivalent_to_only_tags_demangled() { - // R2: --symbols maps to DemangledSymbol only; equivalent to --only-tags demangled. - let flag = stringy() - .args(["tests/fixtures/test_binary_pe.exe", "--symbols", "--json"]) - .output() - .expect("should succeed"); - let tag = stringy() - .args([ - "tests/fixtures/test_binary_pe.exe", - "--only-tags", - "demangled", - "--json", - ]) - .output() - .expect("should succeed"); - assert!(flag.status.success() && tag.status.success()); - assert_eq!( - sorted_stdout_lines(&flag.stdout), - sorted_stdout_lines(&tag.stdout), - "--symbols should produce the same rows as --only-tags demangled" - ); -} - -#[test] -fn cli_convenience_flags_combine_as_union() { - // AE1 / R3: --imports --exports equals repeated --only-tags import export (union). - let flags = stringy() - .args([ - "tests/fixtures/test_binary_pe.exe", - "--imports", - "--exports", - "--json", - ]) - .output() - .expect("should succeed"); - let tags = stringy() - .args([ - "tests/fixtures/test_binary_pe.exe", - "--only-tags", - "import", - "--only-tags", - "export", - "--json", - ]) - .output() - .expect("should succeed"); - assert!(flags.status.success() && tags.status.success()); - assert_eq!( - sorted_stdout_lines(&flags.stdout), - sorted_stdout_lines(&tags.stdout), - "--imports --exports should equal repeated --only-tags import export" - ); -} - -#[test] -fn cli_imports_conflicts_with_only_tags() { - // AE2 / R4: convenience flag conflicts with --only-tags at parse time. - stringy() - .args([ - "tests/fixtures/test_binary_elf", - "--imports", - "--only-tags", - "url", - ]) - .assert() - .failure() - .code(2) - .stderr(predicate::str::contains("cannot be used with")); -} - -#[test] -fn cli_symbols_conflicts_with_raw() { - // AE3 / R4: convenience flag conflicts with --raw at parse time. - stringy() - .args(["tests/fixtures/test_binary_elf", "--symbols", "--raw"]) - .assert() - .failure() - .code(2) - .stderr(predicate::str::contains("cannot be used with")); -} - -#[test] -fn cli_imports_no_tags_contradiction_rejected() { - // AE4 / R5: resolved include set overlapping --no-tags triggers runtime validation. - stringy() - .args([ - "tests/fixtures/test_binary_elf", - "--imports", - "--no-tags", - "import", - ]) - .assert() - .failure() - .stderr(predicate::str::contains("conflicting tag filters")); -} - -#[test] -fn cli_imports_no_tags_compatible() { - // AE5 / R5: no contradiction between --imports and --no-tags version. - stringy() - .args([ - "tests/fixtures/test_binary_elf", - "--imports", - "--no-tags", - "version", - ]) - .assert() - .success(); -} - -#[test] -fn cli_help_lists_convenience_flags() { - // R6: help lists the three flags and each points to --only-tags. - stringy() - .arg("--help") - .assert() - .success() - .stdout(predicate::str::contains("--imports")) - .stdout(predicate::str::contains("--exports")) - .stdout(predicate::str::contains("--symbols")) - .stdout(predicate::str::contains("Shorthand for --only-tags")); -} diff --git a/tests/integration_cli_convenience_flags.rs b/tests/integration_cli_convenience_flags.rs new file mode 100644 index 00000000..f9a547ea --- /dev/null +++ b/tests/integration_cli_convenience_flags.rs @@ -0,0 +1,153 @@ +//! Integration tests for the convenience flags --imports / --exports / +//! --symbols (issue #208). These are shorthands for the corresponding +//! `--only-tags` values and must stay behaviorally equivalent to them. +//! +//! Fixture note: `test_binary_elf` has import=81, export=9, demangled=0 rows. +//! The equivalence tests run against it because both import and export are +//! non-empty there, so an accidental tag mis-mapping (e.g. --symbols routing to +//! Import) is caught. No fixture currently yields demangled-tagged rows, so the +//! --symbols equivalence is additionally guarded by asserting it does not alias +//! --imports/--exports (which do have rows on this fixture). + +use assert_cmd::{Command, cargo_bin_cmd}; +use predicates::prelude::*; + +const ELF: &str = "tests/fixtures/test_binary_elf"; + +fn stringy() -> Command { + cargo_bin_cmd!("stringy") +} + +/// Sorted, non-empty stdout lines so equivalence checks are robust to any +/// run-to-run ordering nondeterminism in extraction or score tie-breaking. +fn sorted_stdout_lines(bytes: &[u8]) -> Vec { + let text = String::from_utf8_lossy(bytes); + let mut lines: Vec = text + .lines() + .filter(|l| !l.is_empty()) + .map(|l| l.to_string()) + .collect(); + lines.sort(); + lines +} + +/// Run `stringy --json ` and return its sorted output rows. +fn elf_rows(extra: &[&str]) -> Vec { + let mut args = vec![ELF, "--json"]; + args.extend_from_slice(extra); + let output = stringy().args(&args).output().expect("stringy should run"); + assert!( + output.status.success(), + "stringy should succeed for args {extra:?}" + ); + sorted_stdout_lines(&output.stdout) +} + +#[test] +fn cli_imports_equivalent_to_only_tags_import() { + // R1: --imports behaves exactly like --only-tags import. + let flag = elf_rows(&["--imports"]); + let tag = elf_rows(&["--only-tags", "import"]); + assert!(!flag.is_empty(), "ELF fixture should yield import rows"); + assert_eq!(flag, tag, "--imports must equal --only-tags import"); +} + +#[test] +fn cli_exports_equivalent_to_only_tags_export() { + // R1: --exports behaves exactly like --only-tags export. + let flag = elf_rows(&["--exports"]); + let tag = elf_rows(&["--only-tags", "export"]); + assert!(!flag.is_empty(), "ELF fixture should yield export rows"); + assert_eq!(flag, tag, "--exports must equal --only-tags export"); +} + +#[test] +fn cli_symbols_maps_to_demangled_only() { + // R2: --symbols maps to DemangledSymbol only. No fixture has demangled rows, + // so equivalence to --only-tags demangled is a (currently empty) match; the + // load-bearing guard is that --symbols does not alias --imports/--exports, + // both of which DO have rows on the ELF fixture. + let symbols = elf_rows(&["--symbols"]); + let demangled = elf_rows(&["--only-tags", "demangled"]); + assert_eq!( + symbols, demangled, + "--symbols must equal --only-tags demangled" + ); + assert_ne!( + symbols, + elf_rows(&["--imports"]), + "--symbols must not alias --imports" + ); + assert_ne!( + symbols, + elf_rows(&["--exports"]), + "--symbols must not alias --exports" + ); +} + +#[test] +fn cli_convenience_flags_combine_as_union() { + // AE1 / R3: --imports --exports is the OR-union, matching repeated --only-tags. + let flags = elf_rows(&["--imports", "--exports"]); + let tags = elf_rows(&["--only-tags", "import", "--only-tags", "export"]); + assert!(!flags.is_empty(), "union should yield import+export rows"); + assert_eq!( + flags, tags, + "--imports --exports must equal repeated --only-tags import export" + ); +} + +#[test] +fn cli_convenience_flags_conflict_with_only_tags_and_raw() { + // AE2 / AE3 / R4: every (flag x conflict-target) pair is rejected by clap. + for flag in ["--imports", "--exports", "--symbols"] { + for conflict in [["--only-tags", "url"].as_slice(), ["--raw"].as_slice()] { + let mut args = vec![ELF, flag]; + args.extend_from_slice(conflict); + stringy() + .args(&args) + .assert() + .failure() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); + } + } +} + +#[test] +fn cli_convenience_flags_no_tags_contradiction_rejected() { + // AE4 / R5: a resolved include tag also excluded via --no-tags fails validation. + for (flag, tag) in [ + ("--imports", "import"), + ("--exports", "export"), + ("--symbols", "demangled"), + ] { + stringy() + .args([ELF, flag, "--no-tags", tag]) + .assert() + .failure() + .stderr(predicate::str::contains("conflicting tag filters")); + } +} + +#[test] +fn cli_imports_no_tags_compatible() { + // AE5 / R5: --imports composes with a non-overlapping --no-tags. + stringy() + .args([ELF, "--imports", "--no-tags", "version"]) + .assert() + .success(); +} + +#[test] +fn cli_help_lists_convenience_flags() { + // R6: help lists the three flags and each points to --only-tags. + stringy() + .arg("--help") + .assert() + .success() + .stdout(predicate::str::contains("--imports")) + .stdout(predicate::str::contains("--exports")) + .stdout(predicate::str::contains("--symbols")) + .stdout(predicate::str::contains("Shorthand for --only-tags")); +} From 45e723b5f14adb3b6f4adc71358f595c1e8616af Mon Sep 17 00:00:00 2001 From: UncleSp1d3r Date: Sat, 4 Jul 2026 23:06:21 -0400 Subject: [PATCH 4/5] test(cli): pin convenience-flag tag mappings and complete conflict docs Add fixture-independent unit tests for Cli::resolved_include_tags() so the --symbols -> DemangledSymbol mapping (and the three-flag union) is falsifiable even though no fixture yields demangled-tagged rows. Strengthen the --help test to assert each flag's --only-tags shorthand line, and parameterize the --no-tags compatibility test across all three flags. Document --raw's full conflict set (now including the convenience flags) in AGENTS.md and the mdBook CLI reference, and clarify the resolved_include_tags doc that the convenience mechanism may combine several flags into a union. Signed-off-by: UncleSp1d3r --- AGENTS.md | 2 +- docs/src/cli.md | 10 ++--- src/main.rs | 49 ++++++++++++++++++++-- tests/integration_cli_convenience_flags.rs | 22 ++++++---- 4 files changed, 66 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c5759249..22357d15 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ Use idiomatic `clap` derive API patterns. Push validation into clap wherever pos | `--min-len` | `-m` | `Option` | Custom parser enforces >= 1 | | `--top` | `-t` | `Option` | Custom parser enforces >= 1 | | `--enc` | | `Option` | utf8, utf16, utf16le, utf16be filter by stored encoding; `ascii` is content-based (matches pure-ASCII text that is not UTF-16, since extraction labels ASCII content as UTF-8) | -| `--raw` | | bool | Conflicts with `--only-tags`, `--no-tags`, `--top`, `--debug`, `--yara` | +| `--raw` | | bool | Conflicts with `--only-tags`, `--no-tags`, `--top`, `--debug`, `--yara`, `--imports`, `--exports`, `--symbols` | | `--summary` | | bool | Conflicts with `--json`, `--yara`; runtime TTY check | | `--debug` | | bool | Conflicts with `--raw` | diff --git a/docs/src/cli.md b/docs/src/cli.md index c20c0c3d..813c0f80 100644 --- a/docs/src/cli.md +++ b/docs/src/cli.md @@ -34,11 +34,11 @@ stringy [OPTIONS] - # read from stdin ### Mode Flags -| Option | Description | -| ----------- | ------------------------------------------------------------------------------------------------------------------------------- | -| `--raw` | Extraction-only mode (no tagging, ranking, or scoring); conflicts with `--only-tags`, `--no-tags`, `--top`, `--debug`, `--yara` | -| `--summary` | Append summary block (TTY table mode only); conflicts with `--json`, `--yara` | -| `--debug` | Include score-breakdown fields (`section_weight`, `semantic_boost`, `noise_penalty`) in JSON output; conflicts with `--raw` | +| Option | Description | +| ----------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--raw` | Extraction-only mode (no tagging, ranking, or scoring); conflicts with `--only-tags`, `--no-tags`, `--top`, `--debug`, `--yara`, `--imports`, `--exports`, `--symbols` | +| `--summary` | Append summary block (TTY table mode only); conflicts with `--json`, `--yara` | +| `--debug` | Include score-breakdown fields (`section_weight`, `semantic_boost`, `noise_penalty`) in JSON output; conflicts with `--raw` | ## Encoding Options diff --git a/src/main.rs b/src/main.rs index 1d3c285e..f44ba379 100644 --- a/src/main.rs +++ b/src/main.rs @@ -165,9 +165,10 @@ struct Cli { impl Cli { /// Effective include-tag set: explicit `--only-tags` plus the convenience /// flags (`--imports` / `--exports` / `--symbols`). The convenience flags - /// conflict with `--only-tags` at parse time, so in practice only one source - /// populates the set, but merging is harmless and keeps a single - /// authoritative include list. + /// conflict with `--only-tags` at parse time, so the set is populated by + /// exactly one of those two mechanisms -- never both -- though the + /// convenience-flag mechanism itself may combine several flags into a union. + /// Merging is harmless and keeps a single authoritative include list. fn resolved_include_tags(&self) -> Vec { let mut tags = self.only_tags.clone(); if self.imports { @@ -322,3 +323,45 @@ fn main() { std::process::exit(e.exit_code()); } } + +#[cfg(test)] +mod tests { + use super::*; + + // Fixture-independent checks that each convenience flag maps to the exact + // Tag it claims. Integration tests can't falsify the --symbols mapping + // because no fixture yields demangled-tagged rows; these pin it directly. + + #[test] + fn resolved_include_tags_imports_is_import_only() { + let cli = Cli::parse_from(["stringy", "f", "--imports"]); + assert_eq!(cli.resolved_include_tags(), vec![Tag::Import]); + } + + #[test] + fn resolved_include_tags_exports_is_export_only() { + let cli = Cli::parse_from(["stringy", "f", "--exports"]); + assert_eq!(cli.resolved_include_tags(), vec![Tag::Export]); + } + + #[test] + fn resolved_include_tags_symbols_is_demangled_only() { + let cli = Cli::parse_from(["stringy", "f", "--symbols"]); + assert_eq!(cli.resolved_include_tags(), vec![Tag::DemangledSymbol]); + } + + #[test] + fn resolved_include_tags_all_three_flags_form_union() { + let cli = Cli::parse_from(["stringy", "f", "--imports", "--exports", "--symbols"]); + assert_eq!( + cli.resolved_include_tags(), + vec![Tag::Import, Tag::Export, Tag::DemangledSymbol] + ); + } + + #[test] + fn resolved_include_tags_empty_without_convenience_flags() { + let cli = Cli::parse_from(["stringy", "f"]); + assert!(cli.resolved_include_tags().is_empty()); + } +} diff --git a/tests/integration_cli_convenience_flags.rs b/tests/integration_cli_convenience_flags.rs index f9a547ea..4f11f3a5 100644 --- a/tests/integration_cli_convenience_flags.rs +++ b/tests/integration_cli_convenience_flags.rs @@ -131,17 +131,19 @@ fn cli_convenience_flags_no_tags_contradiction_rejected() { } #[test] -fn cli_imports_no_tags_compatible() { - // AE5 / R5: --imports composes with a non-overlapping --no-tags. - stringy() - .args([ELF, "--imports", "--no-tags", "version"]) - .assert() - .success(); +fn cli_convenience_flags_no_tags_compatible() { + // AE5 / R5: each convenience flag composes with a non-overlapping --no-tags. + for flag in ["--imports", "--exports", "--symbols"] { + stringy() + .args([ELF, flag, "--no-tags", "version"]) + .assert() + .success(); + } } #[test] fn cli_help_lists_convenience_flags() { - // R6: help lists the three flags and each points to --only-tags. + // R6: help lists the three flags and each one points to --only-tags. stringy() .arg("--help") .assert() @@ -149,5 +151,9 @@ fn cli_help_lists_convenience_flags() { .stdout(predicate::str::contains("--imports")) .stdout(predicate::str::contains("--exports")) .stdout(predicate::str::contains("--symbols")) - .stdout(predicate::str::contains("Shorthand for --only-tags")); + .stdout(predicate::str::contains("Shorthand for --only-tags import")) + .stdout(predicate::str::contains("Shorthand for --only-tags export")) + .stdout(predicate::str::contains( + "Shorthand for --only-tags demangled", + )); } From 7804bbd995656112da3b0b45b91d75f598c6e4b3 Mon Sep 17 00:00:00 2001 From: UncleSp1d3r Date: Sat, 4 Jul 2026 23:21:45 -0400 Subject: [PATCH 5/5] refactor(tags): render tags via clap ValueEnum, fixing Debug leak User-facing messages rendered tags with Debug (`Import`) instead of the CLI form users type (`import`): the --no-tags overlap error and the TTY summary's Top-tags line. Make Tag a clap ValueEnum so the canonical names live once in #[value(name = ...)] attributes; route FromStr through ValueEnum (single source) and add a Display impl that renders the canonical name via PossibleValue. Switch both Debug display sites to Display, and collapse the duplicate 25-arm tag_to_display_string helper (identical output) onto Display so the table, summary, error message, and parser all share one mapping. The arg parsers keep value_parser = ::from_str, so --help and parsing behavior are unchanged; JSON serde serialization is untouched. Signed-off-by: UncleSp1d3r --- src/main.rs | 6 +-- src/output/table/formatting.rs | 39 ++----------------- src/output/table/tty.rs | 2 +- src/types/mod.rs | 70 ++++++++++++++++++++-------------- src/types/tests.rs | 27 ++++++++++++- tests/integration_flows_6_7.rs | 4 +- 6 files changed, 76 insertions(+), 72 deletions(-) diff --git a/src/main.rs b/src/main.rs index f44ba379..1939ed91 100644 --- a/src/main.rs +++ b/src/main.rs @@ -103,7 +103,7 @@ struct Cli { #[arg( long = "only-tags", action = ArgAction::Append, - value_parser = Tag::from_str, + value_parser = ::from_str, value_name = "TAG", long_help = "Include only strings with this tag. Repeat the flag for multiple tags \ (OR logic).\nValid tags: url, domain, ipv4, ipv6, filepath, regpath, guid, email, \ @@ -116,7 +116,7 @@ struct Cli { #[arg( long = "no-tags", action = ArgAction::Append, - value_parser = Tag::from_str, + value_parser = ::from_str, value_name = "TAG", long_help = "Exclude strings with this tag. Repeat the flag for multiple tags \ (OR logic).\nValid tags: url, domain, ipv4, ipv6, filepath, regpath, guid, email, \ @@ -193,7 +193,7 @@ fn run(cli: &Cli) -> Result<(), StringyError> { .filter(|t| cli.no_tags.contains(t)) .collect(); if !overlap.is_empty() { - let tag_names: Vec = overlap.iter().map(|t| format!("{t:?}")).collect(); + let tag_names: Vec = overlap.iter().map(|t| t.to_string()).collect(); return Err(StringyError::ValidationError(format!( "conflicting tag filters: {} are both included and excluded (--no-tags)\n\ Remove these tags from the include filter or from --no-tags to continue.", diff --git a/src/output/table/formatting.rs b/src/output/table/formatting.rs index 9668689f..0e9d1ec9 100644 --- a/src/output/table/formatting.rs +++ b/src/output/table/formatting.rs @@ -56,7 +56,7 @@ pub fn format_tags(tags: &[Tag]) -> String { let tag_strings: Vec = tags .iter() .filter(|tag| tag_boost_value(tag, config) == max_boost) - .map(tag_to_display_string) + .map(|tag| tag.to_string()) .collect(); let result = tag_strings.join(", "); @@ -74,39 +74,6 @@ fn tag_boost_value(tag: &Tag, config: &RankingConfig) -> i32 { config.tag_boosts.get(tag).copied().unwrap_or(0) } -/// Convert a single tag to its display string. -/// -/// Uses the serde rename value where defined, otherwise uses lowercase Debug format. -pub(crate) fn tag_to_display_string(tag: &Tag) -> String { - match tag { - Tag::Url => "url".to_string(), - Tag::Domain => "domain".to_string(), - Tag::IPv4 => "ipv4".to_string(), - Tag::IPv6 => "ipv6".to_string(), - Tag::FilePath => "filepath".to_string(), - Tag::RegistryPath => "regpath".to_string(), - Tag::Guid => "guid".to_string(), - Tag::Email => "email".to_string(), - Tag::Base64 => "b64".to_string(), - Tag::FormatString => "fmt".to_string(), - Tag::UserAgent => "user-agent-ish".to_string(), - Tag::DemangledSymbol => "demangled".to_string(), - Tag::Import => "import".to_string(), - Tag::Export => "export".to_string(), - Tag::Version => "version".to_string(), - Tag::Manifest => "manifest".to_string(), - Tag::Resource => "resource".to_string(), - Tag::DylibPath => "dylib-path".to_string(), - Tag::Rpath => "rpath".to_string(), - Tag::RpathVariable => "rpath-var".to_string(), - Tag::FrameworkPath => "framework-path".to_string(), - Tag::Crypto => "crypto".to_string(), - Tag::Network => "network".to_string(), - Tag::FileIO => "fileio".to_string(), - Tag::EntryPoint => "entry-point".to_string(), - } -} - /// Truncate a string to the specified maximum length. /// /// If the string exceeds the maximum length, it is truncated and `...` is appended. @@ -254,8 +221,8 @@ mod tests { ]; for tag in all_tags { - let display = tag_to_display_string(&tag); - assert!(!display.is_empty(), "Tag {:?} should have display", tag); + let display = tag.to_string(); + assert!(!display.is_empty(), "Tag {tag:?} should have display"); assert!(display.is_ascii(), "Tag display should be ASCII"); } } diff --git a/src/output/table/tty.rs b/src/output/table/tty.rs index 92184d23..2a7eb460 100644 --- a/src/output/table/tty.rs +++ b/src/output/table/tty.rs @@ -142,7 +142,7 @@ fn format_summary_block(metadata: &OutputMetadata) -> String { let tag_parts: Vec = metadata .top_tags .iter() - .map(|(tag, count)| format!("{tag:?}: {count}")) + .map(|(tag, count)| format!("{tag}: {count}")) .collect(); block.push_str(&format!("\nTop tags: {}", tag_parts.join(", "))); } diff --git a/src/types/mod.rs b/src/types/mod.rs index 421ec2e6..88e9b736 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -6,6 +6,7 @@ mod found_string; pub use error::{Result, StringyError}; +use clap::ValueEnum; use serde::{Deserialize, Serialize}; /// Represents the encoding of an extracted string @@ -19,83 +20,96 @@ pub enum Encoding { /// Semantic tags for classifying strings #[non_exhaustive] -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, ValueEnum)] pub enum Tag { + #[value(name = "url")] Url, + #[value(name = "domain")] Domain, #[serde(rename = "ipv4")] + #[value(name = "ipv4")] IPv4, #[serde(rename = "ipv6")] + #[value(name = "ipv6")] IPv6, #[serde(rename = "filepath")] + #[value(name = "filepath")] FilePath, #[serde(rename = "regpath")] + #[value(name = "regpath")] RegistryPath, #[serde(rename = "guid")] + #[value(name = "guid")] Guid, + #[value(name = "email")] Email, #[serde(rename = "b64")] + #[value(name = "b64")] Base64, #[serde(rename = "fmt")] + #[value(name = "fmt")] FormatString, #[serde(rename = "user-agent-ish")] + #[value(name = "user-agent-ish")] UserAgent, #[serde(rename = "demangled")] + #[value(name = "demangled")] DemangledSymbol, + #[value(name = "import")] Import, + #[value(name = "export")] Export, + #[value(name = "version")] Version, + #[value(name = "manifest")] Manifest, + #[value(name = "resource")] Resource, #[serde(rename = "dylib-path")] + #[value(name = "dylib-path")] DylibPath, #[serde(rename = "rpath")] + #[value(name = "rpath")] Rpath, #[serde(rename = "rpath-var")] + #[value(name = "rpath-var")] RpathVariable, #[serde(rename = "framework-path")] + #[value(name = "framework-path")] FrameworkPath, #[serde(rename = "crypto")] + #[value(name = "crypto")] Crypto, #[serde(rename = "network")] + #[value(name = "network")] Network, #[serde(rename = "fileio")] + #[value(name = "fileio")] FileIO, #[serde(rename = "entry-point")] + #[value(name = "entry-point")] EntryPoint, } impl std::str::FromStr for Tag { type Err = String; + /// Parses a canonical CLI tag name into a `Tag`, delegating to the + /// `ValueEnum` definition so the accepted names have a single source of + /// truth (the `#[value(name = ...)]` attributes above). fn from_str(s: &str) -> std::result::Result { - match s { - "url" => Ok(Tag::Url), - "domain" => Ok(Tag::Domain), - "ipv4" => Ok(Tag::IPv4), - "ipv6" => Ok(Tag::IPv6), - "filepath" => Ok(Tag::FilePath), - "regpath" => Ok(Tag::RegistryPath), - "guid" => Ok(Tag::Guid), - "email" => Ok(Tag::Email), - "b64" => Ok(Tag::Base64), - "fmt" => Ok(Tag::FormatString), - "user-agent-ish" => Ok(Tag::UserAgent), - "demangled" => Ok(Tag::DemangledSymbol), - "import" => Ok(Tag::Import), - "export" => Ok(Tag::Export), - "version" => Ok(Tag::Version), - "manifest" => Ok(Tag::Manifest), - "resource" => Ok(Tag::Resource), - "dylib-path" => Ok(Tag::DylibPath), - "rpath" => Ok(Tag::Rpath), - "rpath-var" => Ok(Tag::RpathVariable), - "framework-path" => Ok(Tag::FrameworkPath), - "crypto" => Ok(Tag::Crypto), - "network" => Ok(Tag::Network), - "fileio" => Ok(Tag::FileIO), - "entry-point" => Ok(Tag::EntryPoint), - _ => Err(format!("unknown tag: {s}")), + ::from_str(s, false).map_err(|_| format!("unknown tag: {s}")) + } +} + +impl std::fmt::Display for Tag { + /// Renders the canonical CLI tag name -- the form users type and that + /// `--only-tags`/`--no-tags` accept -- sourced from the `ValueEnum` + /// definition rather than the `Debug` variant name (`import`, not `Import`). + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.to_possible_value() { + Some(value) => f.write_str(value.get_name()), + None => f.write_str(""), } } } diff --git a/src/types/tests.rs b/src/types/tests.rs index be039a3d..d0b10813 100644 --- a/src/types/tests.rs +++ b/src/types/tests.rs @@ -152,7 +152,7 @@ fn test_tag_from_str_all_variants() { ]; for (input, expected) in cases { - let result = Tag::from_str(input); + let result = ::from_str(input); assert_eq!(result, Ok(expected), "failed for input: {input}"); } } @@ -161,6 +161,29 @@ fn test_tag_from_str_all_variants() { fn test_tag_from_str_unknown() { use std::str::FromStr; - let result = Tag::from_str("bogus"); + let result = ::from_str("bogus"); assert_eq!(result, Err("unknown tag: bogus".to_string())); } + +#[test] +fn test_tag_display_renders_cli_name_and_round_trips() { + use clap::ValueEnum; + use std::str::FromStr; + + // Display must render the canonical CLI form (the token users type), not + // the PascalCase Debug variant name, and every rendered name must parse + // back to the same tag. + for tag in Tag::value_variants() { + let name = tag.to_string(); + assert_eq!( + ::from_str(&name), + Ok(tag.clone()), + "Display name {name:?} must parse back to the same tag" + ); + } + + // Spot-check the specific Debug-vs-CLI mismatches this guards against. + assert_eq!(Tag::Import.to_string(), "import"); + assert_eq!(Tag::DemangledSymbol.to_string(), "demangled"); + assert_eq!(Tag::UserAgent.to_string(), "user-agent-ish"); +} diff --git a/tests/integration_flows_6_7.rs b/tests/integration_flows_6_7.rs index f4793a68..a476e9b9 100644 --- a/tests/integration_flows_6_7.rs +++ b/tests/integration_flows_6_7.rs @@ -165,8 +165,8 @@ fn flow7_tty_summary_block_contains_required_fields() { "summary must contain 'Top tags:' label, got: {rendered}" ); assert!( - rendered.contains("Url"), - "summary must contain tag name 'Url', got: {rendered}" + rendered.contains("url"), + "summary must contain the canonical tag name 'url', got: {rendered}" ); // Analysis timing assert!(