morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6784] [CLOSED] fix(pcap): refuse a directory-selected tshark as a tool configuration error #7413

New issue

Closed

opened 2026-11-01 21:47:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:41 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6784
Author: @CassieFox23
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/6433-tshark-directory-config-error

📝 Commits (1)

1592242 fix(pcap): refuse a directory-selected tshark as a tool configuration error

📊 Changes

3 files changed (+38 additions, -5 deletions)

View changed files

📝 src/pcap/PcapConfiguration.ts (+2 -0)
📝 src/pcap/PcapProvider.ts (+12 -5)
📝 tests/boundary/cli/pcapCli.test.ts (+24 -0)

📄 Description

TL;DR: A directory chosen as REA_TSHARK_COMMAND was reported as "unreadable output", with no tool name and no path. Now it gets the same host-tool refusal as a missing tool, so the caller sees what to fix. This fixes the issue, so it says Fixes. Thank you, coygeek, for the exact reproduction and the source trace.

Summary

Fixes #6433. Title: fix(pcap): refuse a directory-selected tshark as a tool configuration error.
When REA_TSHARK_COMMAND (or REA_CAPINFOS_COMMAND) names a directory, REA reported unreadable_output with generic rea doctor advice. The error had no details, no tool name and no path. A missing path or a non-executable file already gets capability_unavailable. Now a directory does too. Thank you, coygeek, for the report.

Problem and expected behavior

Trigger: REA_TSHARK_COMMAND=<a directory> and REA_CAPINFOS_COMMAND=<a regular file>, then inspect-pcap-client. Nothing is launched. REA first reads each tool file to record its identity. That read refuses a directory with an artifact-reader "path" failure. analysisFailure turns that into AnalysisProtocolError, which is shown as unreadable_output.
The issue asks for the existing capability_unavailable family. It should name the tool and the selected path, with the Wireshark advice. The directory is still refused. This is a diagnostic fix, not a way to admit a directory as a tool.

Change and scope

The identity read in PcapProvider.client() now catches an artifact-reader failure with reason path. It rethrows it as PcapConfigurationFailure, the type the missing-tool branch above it already throws. analysisFailure already maps that type to pcapToolUnavailable, so no new mapping is needed. Other reader failures are rethrown unchanged and keep their route. These are cancelled, size limit and changed during read.
Two codes, tshark_not_file and capinfos_not_file, join the internal PcapConfigurationFailureCode type. I searched for that type and its existing members. They appear only in src/pcap/PcapProvider.ts and src/pcap/PcapConfiguration.ts. No public error code, schema or doc changes.
Not included: any change to what is accepted as a tool, the 64 MiB read limit, the version probes, or MCP-specific paths. No doc edit, because no doc names these internal codes or this message.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage:
src/pcap/PcapProvider.ts, client(), at the tool identity read before any version probe.
Contribution scope fit and linked maintainer decision:
Not applicable. A fix inside an existing boundary. It admits nothing new.
CLI and MCP/tool-catalog contract:
No change to any schema or catalog. A directory selection now returns capability_unavailable (category unsupported_provider). The issue names this code, and the missing-tool case already uses it. It is no longer unreadable_output. The CLI is the only surface I ran.
Provider, bridge, target-format, or platform compatibility:
pcap provider only. Run on Linux x64. The reporter used macOS. The failing read is not platform specific (source-derived, not run on macOS).
Evidence, artifact, provenance, or reconstruction contract:
None. The failure happens before any capture or Evidence exists.
Target selection, authority, upstream behavior, and cleanup details (if applicable):
Not applicable. No tool is launched. The refusal comes before the version probes.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs):
None. Three source and test files.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated:
tests/boundary/cli/pcapCli.test.ts: one it.each over tshark and capinfos (2 cases, +24 lines). It selects a temporary directory as that tool. It runs inspect-pcap-client --json through the public CLI. It checks for capability_unavailable, the tool name and path, and the Wireshark advice.

Base reproduction or other evidence:
Executed on main (8f91e6055), Node 24.18.0, Linux. I ran the new test against main's source: 2 failed, 5 passed. Both failures were expected 'unreadable_output' to be 'capability_unavailable'. I also ran the real CLI with the issue's three selections, before and after the fix. These were tshark as a directory, a missing path, and a non-executable file (REA_CAPINFOS_COMMAND set to a regular file). Output trimmed:

Before (main), directory:

{"code":"unreadable_output","category":"execution_failure","message":"Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.","retryable":false,"remediation":{...}}

After (this PR), directory:

{"code":"capability_unavailable","category":"unsupported_provider","message":"The selected tshark executable /tmp/pr4-fx/d is not a regular file. Supply Wireshark 4.x tshark and capinfos on PATH or through absolute REA_TSHARK_COMMAND and REA_CAPINFOS_COMMAND; REA does not install them.","retryable":false,"remediation":{...},"details":{"provider_id":"pcap","operation":"inspect_pcap_client","reason":"The selected tshark executable /tmp/pr4-fx/d is not a regular file."}}

Output for the missing path did not change. Output for the non-executable file did not change either. Both gave capability_unavailable, with details, before and after.

User-visible CLI/MCP output (if applicable):
The CLI output above. MCP was not run.
Remaining proof gaps:
Not run on macOS, where the issue was reported. Not run with a real Wireshark install. A symlink to a directory goes through the same path after realpath, but I did not test it by itself. The test does not assert the details object; the CLI output above shows it present. The 64 MiB size-limit refusal for an oversized tool file is unchanged and untested here.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:focused -- tests/boundary/cli/pcapCli.test.ts, new test against main's source, Node 24.18.0 — 2 failed, 5 passed
same command with this change on Node 22.23.1 — 7 passed
same command with this change on Node 24.18.0 — 7 passed
same command with this change on Node 26.11.1 — 7 passed
npm run test:focused -- with tests/boundary/pcap/pcapProvider.test.ts, pcapProviderCancellation.test.ts, pcapProviderCrossPlatform.test.ts, pcapCommand.test.ts, tests/boundary/mcp/pcapMcp.test.ts, pcapWorkspaceAllocation.test.ts and src/pcap/PcapConfiguration.test.ts, Node 24.18.0 — 7 files, 95 passed, 3 skipped
npm run typecheck — exit 0
npm run lint — exit 0 (0 errors; warnings repo-wide). src/pcap/PcapProvider.ts has the same 11 warnings as on main; the file-length one went from 994 to 1001 lines against a limit of 500. The other touched files add none.
npm run format:check — exit 0
No full suite, test:fast, check:fast or docs:check was run.

Compatibility, safety, and release

Breaking changes or migration steps:
None. A directory was already refused. Only the error code and text change, to the code the issue asks for.
Real Hopper/Ghidra, browser, or OS coverage:
Not applicable. Linux x64 only.
Package or release metadata impact:
None.
Security, privacy, process, or containment review:
None needed. The refusal comes earlier than before and launches nothing.

Author readiness

Human owner: @CassieFox23

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

Test setup

Box: Nobara 44 (Fedora 44 base), Linux 7.2.11
CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
RAM: 32 GB DDR4-3600
Node: 22.23.1, 24.18.0, 26.11.1
Head: 159224260. Base: 8f91e6055.

🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6784 **Author:** [@CassieFox23](https://github.com/CassieFox23) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6433-tshark-directory-config-error` --- ### 📝 Commits (1) - [`1592242`](https://github.com/morluto/rea/commit/159224260e9c83048a61bafacf1a1788c4f6d90b) fix(pcap): refuse a directory-selected tshark as a tool configuration error ### 📊 Changes **3 files changed** (+38 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/pcap/PcapConfiguration.ts` (+2 -0) 📝 `src/pcap/PcapProvider.ts` (+12 -5) 📝 `tests/boundary/cli/pcapCli.test.ts` (+24 -0) </details> ### 📄 Description **TL;DR:** A directory chosen as `REA_TSHARK_COMMAND` was reported as "unreadable output", with no tool name and no path. Now it gets the same host-tool refusal as a missing tool, so the caller sees what to fix. This fixes the issue, so it says Fixes. Thank you, coygeek, for the exact reproduction and the source trace.  ## Summary  Fixes #6433. Title: `fix(pcap): refuse a directory-selected tshark as a tool configuration error`. When `REA_TSHARK_COMMAND` (or `REA_CAPINFOS_COMMAND`) names a directory, REA reported `unreadable_output` with generic `rea doctor` advice. The error had no `details`, no tool name and no path. A missing path or a non-executable file already gets `capability_unavailable`. Now a directory does too. Thank you, coygeek, for the report. ## Problem and expected behavior  Trigger: `REA_TSHARK_COMMAND=<a directory>` and `REA_CAPINFOS_COMMAND=<a regular file>`, then `inspect-pcap-client`. Nothing is launched. REA first reads each tool file to record its identity. That read refuses a directory with an artifact-reader "path" failure. `analysisFailure` turns that into `AnalysisProtocolError`, which is shown as `unreadable_output`. The issue asks for the existing `capability_unavailable` family. It should name the tool and the selected path, with the Wireshark advice. The directory is still refused. This is a diagnostic fix, not a way to admit a directory as a tool. ## Change and scope  The identity read in `PcapProvider.client()` now catches an artifact-reader failure with reason `path`. It rethrows it as `PcapConfigurationFailure`, the type the missing-tool branch above it already throws. `analysisFailure` already maps that type to `pcapToolUnavailable`, so no new mapping is needed. Other reader failures are rethrown unchanged and keep their route. These are cancelled, size limit and changed during read. Two codes, `tshark_not_file` and `capinfos_not_file`, join the internal `PcapConfigurationFailureCode` type. I searched for that type and its existing members. They appear only in `src/pcap/PcapProvider.ts` and `src/pcap/PcapConfiguration.ts`. No public error code, schema or doc changes. Not included: any change to what is accepted as a tool, the 64 MiB read limit, the version probes, or MCP-specific paths. No doc edit, because no doc names these internal codes or this message. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact  - Semantic owner and earliest changed stage: `src/pcap/PcapProvider.ts`, `client()`, at the tool identity read before any version probe. - Contribution scope fit and linked maintainer decision: Not applicable. A fix inside an existing boundary. It admits nothing new. - CLI and MCP/tool-catalog contract: No change to any schema or catalog. A directory selection now returns `capability_unavailable` (category `unsupported_provider`). The issue names this code, and the missing-tool case already uses it. It is no longer `unreadable_output`. The CLI is the only surface I ran. - Provider, bridge, target-format, or platform compatibility: pcap provider only. Run on Linux x64. The reporter used macOS. The failing read is not platform specific (source-derived, not run on macOS). - Evidence, artifact, provenance, or reconstruction contract: None. The failure happens before any capture or Evidence exists. - Target selection, authority, upstream behavior, and cleanup details (if applicable): Not applicable. No tool is launched. The refusal comes before the version probes. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): None. Three source and test files. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage  - Tests added or updated: `tests/boundary/cli/pcapCli.test.ts`: one `it.each` over `tshark` and `capinfos` (2 cases, +24 lines). It selects a temporary directory as that tool. It runs `inspect-pcap-client --json` through the public CLI. It checks for `capability_unavailable`, the tool name and path, and the Wireshark advice. - Base reproduction or other evidence: Executed on main (8f91e6055), Node 24.18.0, Linux. I ran the new test against main's source: 2 failed, 5 passed. Both failures were `expected 'unreadable_output' to be 'capability_unavailable'`. I also ran the real CLI with the issue's three selections, before and after the fix. These were tshark as a directory, a missing path, and a non-executable file (`REA_CAPINFOS_COMMAND` set to a regular file). Output trimmed: Before (main), directory: ``` {"code":"unreadable_output","category":"execution_failure","message":"Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.","retryable":false,"remediation":{...}} ``` After (this PR), directory: ``` {"code":"capability_unavailable","category":"unsupported_provider","message":"The selected tshark executable /tmp/pr4-fx/d is not a regular file. Supply Wireshark 4.x tshark and capinfos on PATH or through absolute REA_TSHARK_COMMAND and REA_CAPINFOS_COMMAND; REA does not install them.","retryable":false,"remediation":{...},"details":{"provider_id":"pcap","operation":"inspect_pcap_client","reason":"The selected tshark executable /tmp/pr4-fx/d is not a regular file."}} ``` Output for the missing path did not change. Output for the non-executable file did not change either. Both gave `capability_unavailable`, with `details`, before and after. - User-visible CLI/MCP output (if applicable): The CLI output above. MCP was not run. - Remaining proof gaps: Not run on macOS, where the issue was reported. Not run with a real Wireshark install. A symlink to a directory goes through the same path after `realpath`, but I did not test it by itself. The test does not assert the `details` object; the CLI output above shows it present. The 64 MiB size-limit refusal for an oversized tool file is unchanged and untested here. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed  - `npm run test:focused -- tests/boundary/cli/pcapCli.test.ts`, new test against main's source, Node 24.18.0 — 2 failed, 5 passed - same command with this change on Node 22.23.1 — 7 passed - same command with this change on Node 24.18.0 — 7 passed - same command with this change on Node 26.11.1 — 7 passed - `npm run test:focused --` with `tests/boundary/pcap/pcapProvider.test.ts`, `pcapProviderCancellation.test.ts`, `pcapProviderCrossPlatform.test.ts`, `pcapCommand.test.ts`, `tests/boundary/mcp/pcapMcp.test.ts`, `pcapWorkspaceAllocation.test.ts` and `src/pcap/PcapConfiguration.test.ts`, Node 24.18.0 — 7 files, 95 passed, 3 skipped - `npm run typecheck` — exit 0 - `npm run lint` — exit 0 (0 errors; warnings repo-wide). `src/pcap/PcapProvider.ts` has the same 11 warnings as on main; the file-length one went from 994 to 1001 lines against a limit of 500. The other touched files add none. - `npm run format:check` — exit 0 - No full suite, `test:fast`, `check:fast` or `docs:check` was run. ## Compatibility, safety, and release  - Breaking changes or migration steps: None. A directory was already refused. Only the error code and text change, to the code the issue asks for.  - Real Hopper/Ghidra, browser, or OS coverage: Not applicable. Linux x64 only. - Package or release metadata impact: None. - Security, privacy, process, or containment review: None needed. The refusal comes earlier than before and launches nothing. ## Author readiness  Human owner: @CassieFox23 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge  - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. <details> <summary>Test setup</summary> - Box: Nobara 44 (Fedora 44 base), Linux 7.2.11 - CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads - RAM: 32 GB DDR4-3600 - Node: 22.23.1, 24.18.0, 26.11.1 - Head: 159224260. Base: 8f91e6055. </details> 🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:41 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7413

No description provided.

Rows
Columns