Repository navigation
feat: Import/Export symbol classification pipeline - #174
Merged
mergify[bot] merged 21 commits intoJun 19, 2026
Merged
mergify[bot] merged 21 commits into
mergify[bot] merged 21 commits into
Conversation
…lity Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…sification Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…sification pipeline Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…sification pipeline Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
The floated toolchain's clippy (1.96) flags unnecessary_sort_by on these pre-existing descending sorts. Convert to sort_by_key(Reverse(..)) -- same stable comparator, behavior-preserving -- so clippy -D warnings passes. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…e source Add four semantic Tag variants (serde renames crypto/network/fileio/entry-point) and a StringSource::SectionName variant for section-name rows. Thread the tags through FromStr, the table and YARA display formatters, and both CLI --only-tags /--no-tags long_help literals. Extend the variant-enumerating test lists and the JSON source-serialization test; regenerate the YARA all-tags snapshot. Implements U1 (R9). Foundational unit for the symbol classification pipeline. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Crypto +50, Network +45, FileIO +35, EntryPoint +40 in RankingConfig::default(). Extend test_default_config_values and add a test that a Crypto-tagged string outranks an otherwise identical untagged peer by at least the boost. Implements U2 (R10). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
New ImportClassifier converts ContainerInfo imports, exports, and section names into tagged FoundStrings: imports carry Tag::Import plus a semantic tag (Crypto/ Network/FileIO) from static LazyLock API sets, with section from library or a format default (.idata/.dynsym/__LINKEDIT) and RVA when present; exports carry Tag::Export with RVA, are demangled (Tag::DemangledSymbol), and gain Tag::EntryPoint for known entry points; section names emit with StringSource::SectionName. extract_symbol_strings composes all three. Symbols are emitted as Encoding::Utf8 so byte-scanned occurrences share the dedup key (KTD7). Provenance tests use hand-built ContainerInfo (KTD1). Implements U3 (R1-R8, R13; AE1-AE3). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… content filter ASCII extraction now emits Encoding::Utf8 (ASCII is a UTF-8 subset) so a byte-scanned occurrence of a symbol name shares the (text, encoding) dedup key with the classifier's occurrence and merges into one row (KTD7, enables AE4). The Encoding::Ascii variant is retained but no longer produced as output (R15). Since no row is labeled Encoding::Ascii anymore, --enc ascii is redefined as a content filter (new EncodingFilter::AsciiContent): it matches rows whose text is pure-ASCII and whose encoding is not UTF-16, preserving the flag's prior narrow-strings-only meaning (R16). The enabled_encodings ASCII entry still gates whether the ASCII byte-scan runs. Implements U6 (R15, R16). Updates extracted-output encoding assertions and the flow1 JSON snapshot (Ascii -> Utf8). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Replace the untagged import/export push in collect_all_strings with a single extract_symbol_strings call inside the include_symbols gate, so symbols reach live output already tagged and the Import/Export and semantic ranking boosts fire. Section-name rows ride the same gate. Scope section-name emission to recognized formats: the unknown/raw fallback's synthetic 'raw-bytes' section is not a real section, so Unknown format skips section-name rows (keeps empty-file output empty and unknown-binary rows all attributed to 'raw-bytes'). Update symbol-property tests to the classifier contract (RVA/section/tags), assert content via SectionData source where section names now also appear, and add an Unknown-format regression test. Implements U4 (R11, R12; AE4). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…:LazyLock The implementation uses std::sync::LazyLock; update the classification docs prose and example to match. Implements U5 (R14). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Evaluate the O(1) UTF-16 encoding guards before the O(n) text.is_ascii() content scan so UTF-16 rows skip the scan. Same boolean result; tightens the config.rs comment to scope the no-longer-Ascii claim to the extraction path. From ce-simplify-code review. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…symbol rows Code review found that demangling exports inside process_exports (during extraction) ran outside the pipeline's catch_unwind guard -- a cpp_demangle panic on a malicious export name would abort the process -- and also caused raw mode to show demangled text. Demangling already happens once in the pipeline's classify_strings (under catch_unwind, skipped in raw mode), so the classifier emission was redundant. Remove it: ImportClassifier is now stateless and only tags (Export + EntryPoint); the pipeline demangles downstream, preserving AE2 end-to-end. This also lets a mangled export share a dedup key with any byte-scanned occurrence of the same raw name. (Deviates from plan KTD3, which predated the knowledge that the pipeline already demangles.) Also: skip empty section names (PE all-null name fields) so no zero-length row reaches output; document the --enc ascii content-filter contract in the AGENTS.md CLI table and GOTCHAS.md; strengthen the --enc ascii CLI test to assert content-based filtering; re-point the export test to the new contract. From ce-code-review (P1 panic-safety, P2 docs, P3 empty-name guard). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
8 tasks
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughArchitecture & Semantic ClassificationThis PR implements a centralized import/export symbol classification pipeline by introducing API ChangesNew Public Types and Functions:
Encoding Reclassification: Binary Format HandlingThe classifier implements format-aware symbol processing:
Semantic ClassificationFour static API-name HashSets enable semantic tagging:
Each import receives Test CoverageThe PR includes 632 passing tests (23 skipped) with comprehensive symbol classification validation:
WalkthroughIntroduces an ChangesImport/Export Symbol Classification Pipeline
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (main.rs)
participant BasicExtractor
participant ImportClassifier
participant RankingEngine
participant FilterEngine
CLI->>BasicExtractor: extract(config, container_info)
BasicExtractor->>ImportClassifier: extract_symbol_strings(container_info)
ImportClassifier->>ImportClassifier: process_imports → Tag::Import + optional Crypto/Network/FileIO
ImportClassifier->>ImportClassifier: process_exports → Tag::Export + optional EntryPoint
ImportClassifier->>ImportClassifier: process_section_names → StringSource::SectionName [non-Unknown]
ImportClassifier-->>BasicExtractor: Vec<FoundString> (tagged)
BasicExtractor->>RankingEngine: rank_strings (boost by Crypto/Network/FileIO/EntryPoint)
RankingEngine-->>BasicExtractor: sorted by score desc (sort_by_key Reverse)
BasicExtractor->>FilterEngine: apply(EncodingFilter::AsciiContent or other)
FilterEngine->>FilterEngine: AsciiContent: keep if !UTF-16 && text.is_ascii()
FilterEngine-->>CLI: filtered FoundStrings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 Full CI must passWonderful, this rule succeeded.All CI checks must pass. This protection prevents manual merges that bypass the merge queue.
🟢 Do not merge outdated PRsWonderful, this rule succeeded.Make sure PRs are within 10 commits of the base branch before merging
|
Commit 2d7d25a removed zig from idiomatic_version_file_enable_tools without adding it to [tools], so mise stopped reading .zig-version (0.15.2) and CI's gen-fixtures step failed with 'zig couldn't exec process'. Re-add zig to the idiomatic list so mise auto-installs the pinned version (exec_auto_install is on); using the existing .zig-version pin keeps fixture layouts and insta snapshots reproducible. Pre-existing break on main, unrelated to the feature. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
There was a problem hiding this comment.
Pull request overview
Implements a symbol-classification emission path that turns parsed imports/exports/section names into tagged FoundStrings, wiring them into extraction so existing ranking boosts (Import/Export and new semantic tags) apply to live output. Also normalizes ASCII extraction output to Encoding::Utf8 and redefines --enc ascii as a content-based filter to keep deduplication stable.
Changes:
- Added
ImportClassifier+extract_symbol_stringsand routed symbol emission through the extractor (imports/exports tagged; section names emitted asStringSource::SectionName). - Introduced four new semantic tags (Crypto/Network/FileIO/EntryPoint) and added ranking boosts + formatter/CLI/test wiring.
- Normalized “ASCII extraction” to emit
Encoding::Utf8and updated--enc asciifiltering semantics + tests/snapshots/docs accordingly.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_deduplication.rs | Makes dedup assertion resilient to new section-name rows by comparing against pre-dedup count. |
| tests/test_ascii_extraction.rs | Updates expectations for ASCII-extracted strings now labeled Encoding::Utf8. |
| tests/snapshots/output_yara_integration__yara_all_tag_types.snap | Snapshot updates to include new tag types in YARA output. |
| tests/snapshots/integration_flows_1_5__flow1_top3_json.snap | Snapshot updates for encoding relabel from Ascii -> Utf8 in JSON output. |
| tests/output_yara_integration.rs | Extends “all tag types” fixture to cover new semantic tags. |
| tests/integration_extraction.rs | Updates encoding assertions; strengthens symbol-property assertions (tags, section/rva presence). |
| tests/integration_cli.rs | Ensures CLI --help lists new canonical tag names. |
| tests/integration_cli_coverage.rs | Adds end-to-end test asserting --enc ascii is content-based. |
| tests/classification_symbols.rs | New unit/integration coverage for import/export/section-name symbol classification and fixture runs. |
| src/types/tests.rs | Extends tag parsing tests to cover new tag variants. |
| src/types/mod.rs | Adds new Tag variants and StringSource::SectionName. |
| src/pipeline/filter.rs | Implements EncodingFilter::AsciiContent behavior and unit tests. |
| src/pipeline/config.rs | Adds EncodingFilter::AsciiContent with documentation. |
| src/output/yara/mod.rs | Maps new tags to canonical YARA tag names. |
| src/output/table/formatting.rs | Adds display strings and test coverage for new tags. |
| src/output/mod.rs | Refactors sort comparator to sort_by_key(Reverse(...)) for clippy compliance. |
| src/output/json.rs | Extends tag/source serialization tests for new tags and SectionName. |
| src/main.rs | Maps --enc ascii to AsciiContent; updates tag lists in clap long_help. |
| src/extraction/tests.rs | Updates extraction tests for UTF-8 labeling + symbol/section-name behavior. |
| src/extraction/dedup/mod.rs | Refactors score sorting to sort_by_key(Reverse(...)). |
| src/extraction/basic_extractor.rs | Replaces untagged import/export emission with extract_symbol_strings. |
| src/extraction/ascii/tests.rs | Updates ASCII extractor tests to expect Encoding::Utf8. |
| src/extraction/ascii/extraction.rs | Changes ASCII extraction output encoding from Ascii to Utf8. |
| src/classification/ranking.rs | Adds boosts for new tags and a regression test asserting boost effect; refactors sort. |
| src/classification/mod.rs | Exposes imports module and re-exports ImportClassifier / extract_symbol_strings. |
| src/classification/imports.rs | New classifier implementation for imports/exports/section names. |
| GOTCHAS.md | Documents the --enc ascii contract change and encoding relabeling. |
| examples/output_formats.rs | Refactors sorting to sort_by_key(Reverse(...)). |
| examples/basic_extraction.rs | Refactors sorting to sort_by_key(Reverse(...)). |
| docs/src/classification.md | Updates docs from once_cell to std::sync::LazyLock. |
| docs/plans/2026-06-19-001-feat-symbol-classification-pipeline-plan.md | Adds the implementation plan and rationale for the pipeline integration. |
| docs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.md | Adds requirements doc (needs alignment with final implemented decisions). |
| CHANGELOG.md | Formatting-only adjustments (blank lines/line wrapping). |
| AI_POLICY.md | Formatting-only adjustments (line wrapping). |
| AGENTS.md | Updates CLI flag notes to document --enc ascii content-based filtering. |
| .mdformat.toml | Removes mdformat exclude list from config (keeps core validation settings). |
| .claude/settings.json | Updates Claude plugin settings metadata under .claude/. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.rs`:
- Line 50: The help text for the `--enc` CLI option needs to be updated to
reflect that the `ascii` value now uses content-based filtering (AsciiContent)
rather than strict encoding matching. Locate the help text definition for the
`enc` parameter in src/main.rs and update it to clarify that `ascii` means
filtering based on narrow ASCII content semantics, not exact encoding
validation, so users understand the actual behavior when they use `--enc ascii`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f1d0548-60be-41fd-b3e8-00a845f07a48
⛔ Files ignored due to path filters (2)
tests/snapshots/integration_flows_1_5__flow1_top3_json.snapis excluded by!**/*.snap,!**/tests/snapshots/**tests/snapshots/output_yara_integration__yara_all_tag_types.snapis excluded by!**/*.snap,!**/tests/snapshots/**
📒 Files selected for processing (35)
.claude/settings.json.mdformat.tomlAGENTS.mdAI_POLICY.mdCHANGELOG.mdGOTCHAS.mddocs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.mddocs/plans/2026-06-19-001-feat-symbol-classification-pipeline-plan.mddocs/src/classification.mdexamples/basic_extraction.rsexamples/output_formats.rssrc/classification/imports.rssrc/classification/mod.rssrc/classification/ranking.rssrc/extraction/ascii/extraction.rssrc/extraction/ascii/tests.rssrc/extraction/basic_extractor.rssrc/extraction/dedup/mod.rssrc/extraction/tests.rssrc/main.rssrc/output/json.rssrc/output/mod.rssrc/output/table/formatting.rssrc/output/yara/mod.rssrc/pipeline/config.rssrc/pipeline/filter.rssrc/types/mod.rssrc/types/tests.rstests/classification_symbols.rstests/integration_cli.rstests/integration_cli_coverage.rstests/integration_extraction.rstests/output_yara_integration.rstests/test_ascii_extraction.rstests/test_deduplication.rs
💤 Files with no reviewable changes (1)
- .mdformat.toml
…rsion file settings Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
coderabbitai
Bot
added
the
documentation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mise.toml`:
- Line 47: Add "zig" to the idiomatic_version_file_enable_tools array (which
currently contains "python", "rust", and "bun") so that mise will honor the
.zig-version file. Additionally, replace the "latest" version value for zig with
a specific pinned version number to ensure reproducible builds and prevent
silent breaks from version-dependent binary layout changes.
- Line 42: The zig tool is configured to use "latest" version on line 42, but
zig is missing from the idiomatic_version_file_enable_tools list on line 47,
causing mise to ignore the existing .zig-version file pinned to 0.15.2. Add zig
to the idiomatic_version_file_enable_tools list (alongside other tools like
python, rust, and bun that follow the same pattern) to enable mise to respect
the .zig-version file and ensure deterministic fixture builds and snapshot
stability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c36a8426-3022-41f0-afbc-05e791745e3d
⛔ Files ignored due to path filters (1)
mise.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
mise.toml
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mise.toml`:
- Line 47: Add "zig" to the idiomatic_version_file_enable_tools array (which
currently contains "python", "rust", and "bun") so that mise will honor the
.zig-version file. Additionally, replace the "latest" version value for zig with
a specific pinned version number to ensure reproducible builds and prevent
silent breaks from version-dependent binary layout changes.
- Line 42: The zig tool is configured to use "latest" version on line 42, but
zig is missing from the idiomatic_version_file_enable_tools list on line 47,
causing mise to ignore the existing .zig-version file pinned to 0.15.2. Add zig
to the idiomatic_version_file_enable_tools list (alongside other tools like
python, rust, and bun that follow the same pattern) to enable mise to respect
the .zig-version file and ensure deterministic fixture builds and snapshot
stability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c36a8426-3022-41f0-afbc-05e791745e3d
⛔ Files ignored due to path filters (1)
mise.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
mise.toml
🛑 Comments failed to post (2)
mise.toml (2)
42-42:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
# Check if mise.toml exists and view the relevant section if [ -f mise.toml ]; then echo "=== mise.toml content ===" cat -n mise.toml else echo "mise.toml not found" fiRepository: EvilBit-Labs/Stringy
Length of output: 3434
🏁 Script executed:
# Check for any .mise.toml or .tool-versions files that might contain zig version find . -maxdepth 2 -type f \( -name ".mise.toml" -o -name ".tool-versions" -o -name "mise.lock" \) 2>/dev/nullRepository: EvilBit-Labs/Stringy
Length of output: 76
🏁 Script executed:
# Search for references to zig version in justfile and related docs rg -i "zig.*version|gen-fixtures" --type-list | head -20 rg -i "zig.*version|gen-fixtures" -lRepository: EvilBit-Labs/Stringy
Length of output: 705
🏁 Script executed:
# Check GOTCHAS.md for any zig/reproducibility guidance if [ -f GOTCHAS.md ]; then rg -i "zig|reproducib|snapshot|fixture" GOTCHAS.md -A 2 fiRepository: EvilBit-Labs/Stringy
Length of output: 1592
🏁 Script executed:
# Check for version files (.zig-version, .python-version, .rust-version, etc.) find . -maxdepth 1 -type f \( -name ".zig-version" -o -name ".python-version" -o -name ".rust-version" -o -name ".bun-version" \)Repository: EvilBit-Labs/Stringy
Length of output: 97
🏁 Script executed:
# Check the justfile for gen-fixtures to understand zig usage rg -A 10 "gen-fixtures" justfileRepository: EvilBit-Labs/Stringy
Length of output: 2539
🏁 Script executed:
# Check mise.lock to see if zig version is captured there head -50 mise.lockRepository: EvilBit-Labs/Stringy
Length of output: 2716
🏁 Script executed:
# Check the .zig-version file cat .zig-versionRepository: EvilBit-Labs/Stringy
Length of output: 71
🏁 Script executed:
# Check if zig is mentioned elsewhere in documentation about version management rg -i "zig.*version" docs/ -A 2Repository: EvilBit-Labs/Stringy
Length of output: 46
Enable zig version file support in mise configuration
Line 42 sets
zig = "latest"but line 47 does not includeziginidiomatic_version_file_enable_tools. This causes mise to ignore the existing.zig-versionfile (pinned to0.15.2), making fixture builds non-deterministic and breaking snapshot stability. The.zig-versionfile already exists; addzigto the enable list to honor it.Suggested fix
-idiomatic_version_file_enable_tools = [ "python", "rust", "bun" ] +idiomatic_version_file_enable_tools = [ "python", "rust", "bun", "zig" ]The project already maintains
.zig-version = 0.15.2and documentation confirms that zig version changes break insta snapshots. Enabling idiomatic version file support aligns zig with python, rust, and bun (all set to"latest"with version-file backing).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mise.toml` at line 42, The zig tool is configured to use "latest" version on line 42, but zig is missing from the idiomatic_version_file_enable_tools list on line 47, causing mise to ignore the existing .zig-version file pinned to 0.15.2. Add zig to the idiomatic_version_file_enable_tools list (alongside other tools like python, rust, and bun that follow the same pattern) to enable mise to respect the .zig-version file and ensure deterministic fixture builds and snapshot stability.Sources: Coding guidelines, Learnings
47-47:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail echo "Check current mise zig config:" rg -n '^\s*zig\s*=|idiomatic_version_file_enable_tools' mise.toml echo echo "Search for zig version files / declarations in repo:" fd -HI '(^\.zig-version$|zig-version|\.tool-versions$|mise\.toml$)' rg -n '(^|\s)zig(\s|=)' .tool-versions mise.toml 2>/dev/null || true echo echo "Cross-check docs expecting pinned zig behavior:" rg -n 'Zig version|pinned in `mise.toml`|breaking insta snapshots|zig cc'Repository: EvilBit-Labs/Stringy
Length of output: 2799
Add
zigtoidiomatic_version_file_enable_toolsand pin its versionZig is critical for cross-compiling test fixtures (ELF, PE, Mach-O), yet it's absent from the idiomatic version-file tool list on line 47. A
.zig-versionfile exists but won't be honored by mise without this entry. Additionally, Zig is currently set to"latest"(line 42), which contradicts the documented requirement that "Zig version is pinned inmise.toml" (tests/fixtures/README.md:137). Unpinned Zig versions can alter compiled binary layouts and silently break insta snapshots.Add
"zig"to theidiomatic_version_file_enable_toolsarray, and update Zig to a pinned version instead of"latest"to ensure reproducible builds across CI and contributor machines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mise.toml` at line 47, Add "zig" to the idiomatic_version_file_enable_tools array (which currently contains "python", "rust", and "bun") so that mise will honor the .zig-version file. Additionally, replace the "latest" version value for zig with a specific pinned version number to ensure reproducible builds and prevent silent breaks from version-dependent binary layout changes.Source: Coding guidelines
zig = latest pulled 0.16.0, which generates a materially different ELF fixture (81 imports vs 30) than the committed insta snapshots, which were calibrated for the .zig-version pin (0.15.2). Pin zig to 0.15.2 in [tools] so CI-generated fixtures match the snapshots and the version is reproducible across zig releases. Resolves the elf_symbol_extraction snapshot failure. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
CI runs 'mise install --locked', which requires every pinned tool version in mise.lock. The lockfile still held zig 0.16.0 (from the earlier zig=latest), so pinning mise.toml to 0.15.2 failed to resolve under --locked. Relock zig to 0.15.2 for all 9 target platforms and prune the stale 0.16.0 entry so the pin installs cleanly on every CI runner. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- Mark StringSource #[non_exhaustive] (matches Tag) so future variants are not semver-breaking for external exhaustive matches (Copilot) - Clarify --enc help text: ascii is narrow-ASCII content filtering (CodeRabbit) - Soften imports.rs module doc: demangle adds DemangledSymbol on success; names that fail to demangle are left unchanged (Copilot) - Update origin requirements doc R3/R6/R8 to match the implementation: Mach-O __LINKEDIT default, demangle-in-pipeline note, StringSource::SectionName (Copilot) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.md`:
- Around line 38-45: The requirements R1, R5, and R8 describe FoundString
emission for imports, exports, and section names but do not explicitly specify
the encoding format as Encoding::Utf8. Add clarification to the requirements
document by either adding a note to each of R1, R5, and R8 stating that symbol
strings are emitted with Encoding::Utf8, or by creating a new dedicated
requirement (e.g., "Encoding") that documents this constraint applies to all
symbol strings to enable deduplication with byte-scanned occurrences. This
ensures the implementation contract for encoding is clearly documented and
prevents confusion during implementation.
- Line 45: Requirement R8 specifies that section names emit as FoundStrings with
StringSource::SectionName but does not document that section-name rows are
filtered out for BinaryFormat::Unknown format. The synthetic 'raw-bytes' section
used as fallback for Unknown format carries no analytic value and would pollute
output, so this filtering behavior needs to be explicitly documented. Add a
clarification sentence to R8 stating that section-name rows are emitted only for
recognized container formats (BinaryFormat::Elf, BinaryFormat::Pe,
BinaryFormat::MachO) and excluded for BinaryFormat::Unknown, with the rationale
that the Unknown format's synthetic section has no semantic value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d45ba91-7dad-4a3b-9b2f-6e5e2f904197
⛔ Files ignored due to path filters (1)
mise.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
docs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.mdsrc/classification/imports.rssrc/main.rssrc/types/mod.rs
Comment on lines
+38
to
+45
| - R1. Import names emit as `FoundString`s with `source: StringSource::ImportName`, always carrying `Tag::Import`, at `confidence: 1.0`. | ||
| - R2. Imports whose name matches the crypto, network, or file-I/O API sets additionally carry `Tag::Crypto`, `Tag::Network`, or `Tag::FileIO`; imports that match none carry only `Tag::Import`. | ||
| - R3. An import's originating library populates the `section` field when present; when absent, a format-appropriate default is used (`.idata` for PE, `.dynsym` for ELF, `__LINKEDIT` for Mach-O -- Mach-O imports never carry a library, so they always use the default). | ||
| - R4. An import's RVA populates from `ImportInfo::address` when present (it is optional). | ||
| - R5. Export names emit as `FoundString`s with `source: StringSource::ExportName`, always carrying `Tag::Export`, with RVA from `ExportInfo::address` (always available). | ||
| - R6. Exports are run through the existing `SymbolDemangler`; successfully demangled exports additionally carry `Tag::DemangledSymbol`. (Implementation note: demangling runs in the pipeline's `classify_strings` -- under `catch_unwind` and skipped in raw mode -- not in the classifier, so a third-party demangler panic never aborts extraction.) | ||
| - R7. Exports whose name is a known entry point (`main`, `_start`, `DllMain`, `WinMain`, `wWinMain`) additionally carry `Tag::EntryPoint`. | ||
| - R8. Section names emit as standalone `FoundString`s with `source: StringSource::SectionName` (a dedicated variant, distinct from `SectionData`, so section-name rows are distinguishable from byte-scanned section content) at `confidence: 1.0`, ranked alongside other output. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Clarify symbol encoding in requirements.
Requirements R1–R5 describe FoundString emission for imports, exports, and section names but do not specify Encoding::Utf8 as the emission format. The PR summary states "ASCII extraction now emits Encoding::Utf8 (ASCII is a UTF-8 subset), enabling deduplication," and the context shows all classifier symbol strings are constructed with Encoding::Utf8. This constraint should be explicitly stated in the requirements—either in each requirement (R1, R5, R8) or in a single "Encoding" section—to prevent implementation confusion and document the deduplication contract.
Add a note in each of R1, R5, and R8, or add a new requirement line (e.g., "Symbol strings are emitted with Encoding::Utf8 to enable deduplication with byte-scanned occurrences"), documenting this non-obvious constraint.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.md`
around lines 38 - 45, The requirements R1, R5, and R8 describe FoundString
emission for imports, exports, and section names but do not explicitly specify
the encoding format as Encoding::Utf8. Add clarification to the requirements
document by either adding a note to each of R1, R5, and R8 stating that symbol
strings are emitted with Encoding::Utf8, or by creating a new dedicated
requirement (e.g., "Encoding") that documents this constraint applies to all
symbol strings to enable deduplication with byte-scanned occurrences. This
ensures the implementation contract for encoding is clearly documented and
prevents confusion during implementation.
| - R5. Export names emit as `FoundString`s with `source: StringSource::ExportName`, always carrying `Tag::Export`, with RVA from `ExportInfo::address` (always available). | ||
| - R6. Exports are run through the existing `SymbolDemangler`; successfully demangled exports additionally carry `Tag::DemangledSymbol`. (Implementation note: demangling runs in the pipeline's `classify_strings` -- under `catch_unwind` and skipped in raw mode -- not in the classifier, so a third-party demangler panic never aborts extraction.) | ||
| - R7. Exports whose name is a known entry point (`main`, `_start`, `DllMain`, `WinMain`, `wWinMain`) additionally carry `Tag::EntryPoint`. | ||
| - R8. Section names emit as standalone `FoundString`s with `source: StringSource::SectionName` (a dedicated variant, distinct from `SectionData`, so section-name rows are distinguishable from byte-scanned section content) at `confidence: 1.0`, ranked alongside other output. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Document section-name filtering for the Unknown format.
R8 specifies that section names emit as StringSource::SectionName ranked strings, but does not mention that section-name rows are filtered out for BinaryFormat::Unknown. The implementation context shows: "The unknown/raw fallback uses a synthetic 'raw-bytes' section that carries no analytic value (and would pollute empty-file output), so skip section-name rows for Unknown format." This filtering rule should be documented in R8 or as a separate clarification to prevent test/output mismatches.
Add a clarification line to R8, e.g.: "Section-name rows are emitted only for recognized container formats (BinaryFormat::Elf, BinaryFormat::Pe, BinaryFormat::MachO); BinaryFormat::Unknown is excluded because its synthetic 'raw-bytes' section carries no semantic value."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.md`
at line 45, Requirement R8 specifies that section names emit as FoundStrings
with StringSource::SectionName but does not document that section-name rows are
filtered out for BinaryFormat::Unknown format. The synthetic 'raw-bytes' section
used as fallback for Unknown format carries no analytic value and would pollute
output, so this filtering behavior needs to be explicitly documented. Add a
clarification sentence to R8 stating that section-name rows are emitted only for
recognized container formats (BinaryFormat::Elf, BinaryFormat::Pe,
BinaryFormat::MachO) and excluded for BinaryFormat::Unknown, with the rationale
that the Unknown format's synthetic section has no semantic value.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
|
Queued — the merge queue status continues in this comment ↓. |
Member
Author
|
@Mergifyio queue |
Contributor
Merge Queue Status
This pull request spent 12 minutes 9 seconds in the queue, including 11 minutes 47 seconds running CI. Required conditions to merge
|
50 of 54 tasks
mergify
Bot
deleted the
20-implement-importexport-symbol-classification-pipeline
branch
June 19, 2026 22:00
19 tasks
This was referenced Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the import/export symbol classification pipeline (issue #20). A new
ImportClassifierconvertsContainerInfoimports, exports, and section names into tagged, scoredFoundStrings, wired into the extraction path as the single symbol emission point — so the previously-dormantTag::Import/Tag::Exportranking boosts finally fire on live output. Adds four semantic tags (Crypto/Network/FileIO/EntryPoint) recognized via static API-name sets and the existing demangler.Origin plan:
docs/plans/2026-06-19-001-feat-symbol-classification-pipeline-plan.md(fromdocs/brainstorms/2026-06-19-symbol-classification-pipeline-requirements.md).What changed (by implementation unit)
Tagvariants (Crypto/Network/FileIO/EntryPoint, serdecrypto/network/fileio/entry-point) and aStringSource::SectionNamevariant, threaded throughFromStr, the table + YARA display formatters, both CLIlong_helpliterals, and every variant-enumerating test list.RankingConfigboosts: Crypto +50, Network +45, FileIO +35, EntryPoint +40.src/classification/imports.rs:ImportClassifier+extract_symbol_strings. Imports carryTag::Importplus a semantic tag fromLazyLock<HashSet>API sets, withsectionfrom library or a format default (.idata/.dynsym/__LINKEDIT) and RVA when present; exports carryTag::Export+ RVA +Tag::EntryPointfor known entry points; section names emit withStringSource::SectionName.Encoding::Utf8(ASCII is a UTF-8 subset) so a byte-scanned occurrence of a symbol shares the(text, encoding)dedup key and merges.--enc asciiis redefined as a content filter (EncodingFilter::AsciiContent).extract_symbol_stringsintocollect_all_stringsas the single emission point.once_cell→std::sync::LazyLock.Live verification on the ELF fixture: imports tag as
['Import','fileio'](e.g.read/write) with library sections, exports_start/maintag as['Export','entry-point'], section names emit, and there are zero duplicate(text,encoding)keys (AE4 merge confirmed).Notable decisions
--enc asciiis now a content filter (R16, CLI contract change). Since extraction no longer labels rowsEncoding::Ascii,--enc asciimatches rows whose text is pure-ASCII and whose encoding is not UTF-16 — preserving the flag's prior "narrow ASCII strings" meaning. JSON output no longer emits"encoding":"Ascii"(now"Utf8"). Documented in the AGENTS.md CLI table and GOTCHAS.md.raw-bytessection is not a real section, so Unknown format skips section-name rows (keeps empty-file output empty; unknown-binary rows all attributed toraw-bytes).catch_unwindguard (acpp_demanglepanic on a malicious export name could abort the process) and made raw mode show demangled text. The pipeline'sclassify_stringsalready demangles every mangled symbol undercatch_unwind(skipped in raw mode), so the classifier emission was redundant. The classifier now only tags; demangling happens downstream, preserving AE2 end-to-end. KTD3 predated the knowledge that the pipeline already demangles.Test plan
cargo nextest run— 632 pass, 23 skippedjust lint-rust— clean (clippy -D warnings)tests/classification_symbols.rscovers R1-R8, AE1-AE3, the Unknown-format skip, and end-to-end fixture parsing (ELF/PE/Mach-O)--enc asciicontent-filter semantics asserted end-to-end (encoding_filter_ascii_is_content_based)flow1_top3_json,yara_all_tag_types)Residual review findings (not blocking; for maintainer judgment)
source/section/rvaof a nondeterministic first occurrence. Fixing it properly means preferring theImportName/ExportNameoccurrence indedup's representative selection, which is out of scope per the plan's Scope Boundaries ("Redesigning deduplication is out of scope"). Partially documented in KTD1. Suggested follow-up.StringSourceis not#[non_exhaustive]. AddingSectionNameis technically an exhaustive-match break for external crates that matchStringSource(theTagenum is#[non_exhaustive];StringSourceis not). Low real impact at v0.1.0. Consider adding#[non_exhaustive]toStringSourcefor consistency withTag— a public-API policy call left to the maintainer.https://claude.ai/code/session_012sGrToJP47o1ZcGzNKGHt8