Skip to content

fix(pcap): report a refused tool launch as an unavailable tool - #10553

Merged
morluto merged 8 commits into
mainfrom
fix/launch-refusal-classifier
Nov 15, 2026
Merged

morluto merged 8 commits into
mainfrom
fix/launch-refusal-classifier

Conversation

@morluto

@morluto morluto commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

Summary

A refused packet-tool launch could be reported as unreadable output, and matching an errno without its origin could blame TShark when the failure came from preparation or the Linux limiter. This change shares launch-refusal classification across pcap, apktool, GDB, ADB, and Java probes. Pcap uses the spawn error's path and syscall to attribute a direct refusal to the selected tool or limiter and supplies the corresponding repair guidance.

On Linux the tool runs through util-linux prlimit, which sets limits and then replaces itself with the tool. attributePcapLimitedExit (owned by PcapNativeBound, which selects and validates prlimit) reads prlimit's own err(3) diagnostics: prlimit: failed to execute ... with exit 127/126 is a missing or non-executable selected tool, any other prlimit: ... exit 1 is a limiter failure with the limiter repair, and everything else is the tool's own exit, classified exactly as without a limiter. A non-capture file therefore reports unsupported_target on Linux as on other platforms. Only a failure observed before any exit (stream collection failure) keeps the stage unknown. No new process authority or schema is introduced.

Validation

  • Focused pcap, shared launch-classifier, and Ghidra Java-probe tests: 67 passed, including direct spawn failures and an actual child process whose unsuccessful exit leaves attribution unknown.
  • npm run check:fast and formatting checks passed.
  • The launch refusals were injected at the launcher boundary; no operating-system policy refusal or real Linux prlimit/TShark run was performed.

No issue was filed; the problem was found during an error-boundary audit.

  • Review follow-up (maintainer decision: keep platforms consistent): reverted the Linux-only provider expectations to unsupported_target and added prlimit stand-in tests that run on every POSIX host (tool exit attributed to the tool, failed exec attributed to the selected tool, prlimit's own refusal attributed to the limiter). npm run test:focused -- tests/boundary/pcap/pcapCommand.test.ts — 15 passed; the 3 new tests fail on the previous head. Pre-push npm run check:fast passed. The real-prlimit path is covered by the Linux-only pcapProvider tests in CI.
  • Review follow-up (launch identity): src/process/launchFailure.ts now owns which executable a launcher refused (executableLaunchRefusalOf): Node's spawn <path> errors and the Windows native launcher's executable-launch constraint with requestedPath. spawnOwnedProviderProcess gives errnos Node throws synchronously with a bare spawn syscall (ENOEXEC, ENOTDIR, ... — probed on Node 24.22.1/darwin) the same shape. Pcap no longer reads syscall/path itself. npm run test:focused -- src/process/launchFailure.test.ts tests/boundary/pcap/pcapCommand.test.ts src/process/ProviderProcess.launcherExit.test.ts src/windows/WindowsOwnedProcess.test.ts — 66 passed; the real-ENOEXEC and Windows-shaped pcap tests fail without the fix. npm run check:fast passed. Not run on a real Windows host.
  • Review follow-up (Linux ENOEXEC): the real-process synchronous-refusal test now uses ENOTDIR (a path through a regular file), which every POSIX host refuses synchronously; ENOEXEC may instead be retried through /bin/sh on Linux. npm run test:focused -- tests/boundary/pcap/pcapCommand.test.ts — 17 passed; the test fails with nameSpawnedExecutable removed. Not run on Linux locally.

Author readiness

Human owner:

  • 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.

When a capture read's tool launch was refused with EPERM, pcap reported it
as unreadable tool output (AnalysisProtocolError) instead of an unavailable
tool. Pcap also never told "not executable" apart from "missing".

Add classifyLaunchFailure in src/process as the one owner of which spawn
errnos mean missing (ENOENT, ENOTDIR) and which mean present but not
executable (EACCES, EPERM, ENOEXEC, EISDIR). Pcap, apktool and gdb used three
different errno lists; all three now use it.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-15T01:48:43.178873Z 49732b1 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5 Tier: plus

[Medium impact] The reviewed follow-up appears safe to merge; the previous Linux test failure is addressed.

Summary

This PR shares launch-refusal classification across providers and gives pcap errors repair advice for the selected tool or limiter.

  • Pcap errors now name the tool or limiter that failed.
  • Tool probes now share clear categories for launch refusals.

Reviews (7) · Last reviewed commit: "test(pcap): prove synchronous launch nam..." · Reviewed by Greptile

Comment thread src/pcap/PcapCommand.ts Resolved
Comment thread src/pcap/PcapCommand.ts Resolved
…red owner (#10598)

The Ghidra and JADX Java probes and adb's version probe now take
missing-versus-not-executable from classifyLaunchFailure instead of their
own errno checks. ELOOP and ENAMETOOLONG join the owner's missing set, so
adb keeps its existing classification and both Java probes now treat a
non-directory path component as a missing runtime.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b35bb6b1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/pcap/PcapCommand.ts Resolved
@morluto

morluto commented Nov 15, 2026

Copy link
Copy Markdown
Owner Author

Follow-up to the resolved pcap provider thread: per the maintainer's decision to keep platforms consistent, d3f2880 replaces the unknown-stage result with attribution through the prlimit wrapper. attributePcapLimitedExit in PcapNativeBound reads prlimit's own err(3) diagnostics (exit 127/126 with failed to execute means the selected tool is missing or not executable; any other prlimit: exit 1 is a limiter failure); every other exit is the tool's own. The provider tests again expect unsupported_target for a non-capture file. New prlimit stand-in tests in pcapCommand.test.ts cover all three stages on every POSIX host and fail on the previous head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3f288002e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/pcap/PcapCommand.ts Resolved
Name the executable on errnos Node throws synchronously from spawn
(ENOEXEC, ENOTDIR, ...) and read the Windows native launcher's
executable-launch constraint, so pcap attributes those refusals to the
selected tool or limiter instead of an unknown stage.
Comment thread tests/boundary/pcap/pcapCommand.test.ts Resolved
Comment thread src/process/ProviderProcess.ts Resolved
@morluto
morluto merged commit 44fedc8 into main Nov 15, 2026
47 of 50 checks passed
@morluto
morluto deleted the fix/launch-refusal-classifier branch November 15, 2026 02:12
morluto added a commit that referenced this pull request Nov 15, 2026
…sal owner superseded

#10553 moved the last caller of isUnresolvedPathErrorCode onto the shared
launch-failure classifier, leaving the export unused. knip in the deep
static checks lane fails on it, keeping main red. The errno set stays: the
selected-regular-file predicate still uses it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants