morluto/rea

Fork 0

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

[PR #3643] [CLOSED] feat(captures): decrypt TLS in recorded packet captures with a caller-supplied key log #5862

New issue

Closed

opened 2026-10-31 01:10:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3643
Author: @Y0UW1SH
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: feat/3594-tls-key-log-packet-capture

📝 Commits (10+)

c59840d feat(captures): decrypt TLS in recorded packet captures with a caller-supplied key log
5ee2928 fix(captures): preserve TLS key-log lifecycle through upstream sync
cd52910 fix(process): retain exact kernel-limit descriptor cleanup owner
c2aea8f fix(ci): resolve upstream formatting gate after sync
2737cf5 test(verify): isolate Ghidra prerequisite rejection from host tools
b34b1bf test(apple): align archive NaN assertions with current contracts
1aaa504 chore: sync main and reconcile equivalent fixture fixes
7a38d99 test(process): scope deadline watchdog to active execution
fe2d688 fix(captures): withhold nested decrypted command output
2b332cf test(cli): isolate module-mocked command finalization

📊 Changes

22 files changed (+3242 additions, -102 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/SKILL.md (+7 -1)
📝 .github/workflows/real-packet-capture.yml (+3 -1)
📝 docs/packet-captures.md (+112 -0)
➕ scripts/fixtures/tls-key-log-capture.mjs (+204 -0)
📝 scripts/verify-packet-capture.mjs (+250 -18)
➕ src/application/captures/PacketCaptureService.test.ts (+180 -0)
📝 src/application/captures/PacketCaptureService.ts (+17 -9)
📝 src/captures/TsharkCaptureProvider.ts (+231 -44)
📝 src/cli/captureCommands.ts (+7 -0)
📝 src/contracts/captures/captureToolContracts.ts (+9 -1)
➕ src/domain/captures/packetCapture.test.ts (+315 -0)
📝 src/domain/captures/packetCapture.ts (+263 -8)
➕ src/domain/captures/tlsKeyLog.test.ts (+188 -0)
➕ src/domain/captures/tlsKeyLog.ts (+210 -0)
📝 src/process/ArtifactAnalysisResources.test.ts (+49 -0)
📝 src/process/ArtifactAnalysisResources.ts (+47 -9)
📝 src/process/capture/ProcessCaptureLifecycle.test.ts (+27 -1)
📝 tests/boundary/captures/captureProvider.test.ts (+325 -3)
➕ tests/boundary/captures/tsharkKeyLogLifecycle.test.ts (+664 -0)
📝 tests/boundary/cli/captureCleanup.test.ts (+46 -0)

...and 2 more files

📄 Description

Summary

Recorded HTTPS inspection needs matching client session secrets to return decrypted fields. Add optional tls_key_log_path for a key log produced by the caller's own client. TShark receives a private read-only copy; REA returns its identity/counts and selected reviewed metadata, removes transport credentials and URL userinfo, and withholds raw producer diagnostics.

Current head: 6fd11bd. The latest file-close error leak is fixed and pushed; 141 focused tests and static checks pass locally. All 43 reported checks passed. Automated reviews completed; the sole unresolved review thread is the maintainer scope decision. Merging remains blocked by the explicit maintainer scope decision in #3594 required by the changes-requested review. Review readiness and CI success do not supply scope approval.

Refs #3594. Related TShark selection work: #3403 / #3407. inspect_pcap_capture is unchanged.

Implementation and follow-up fixes

Read the caller's key log through the stable artifact reader, bounded to 32 MiB. Report identity, line/label counts and distinct client-random counts without secrets, line text or random values. Counts do not prove decryption; unrecognized lines remain available to TShark.
Pass a private mode-0400 copy as one tls.keylog_file argument, verify private input integrity after execution, and attempt prompt deletion. Retain exact cleanup ownership across failures. Completed redacted Evidence survives late cancellation and workspace cleanup failure.
Admit only the guide's explicit 30 packet/HTTP metadata fields for key-log requests. Named authorization/cookie fields, including supported @ variants, are admitted only for removal. Refuse generic headers, decoded authentication, payload/raw selections, unreviewed fields and every non-null display filter before provider work.
Remove whole selected URI/authority fields containing structural userinfo and record removal. Require selected request methods with HTTP request URIs; refuse uncertain counts and mixed CONNECT/other contexts. Preserve origin-form paths, query data and opaque URI data without guessing secrecy. Validate returned reports against the same boundary.
Withhold decrypting stdout/stderr throughout actual errors, including nested command wrappers. Preserve sizes, exact cancellation reason and physical process facts; keep cleanup supervisors private.
Latest fix: both key-log reads mark their exact failure and cleanup leases confidential. Public cleanup errors omit raw wrappers that retain completed key-log bytes and readable file handles. The same private leases retain cleanup authority through failed admission retries and close(); successful retry clears the original errors' pending state. Ordinary artifact reads retain their existing behavior.
The portable WASM ordering failure reproduced on clean upstream 847472a; upstream independently fixed it in 8cbaf6c. This branch includes the exact upstream isolated test through the sync with main bb212ff, retaining all five assertions.
Contracts, CLI/MCP coverage, guide and shipped skill describe the narrowed behavior. The real verifier includes all selected frames, a credential-bearing absolute URI, generic-field/filter refusals and an admitted producer-failure request.

The previous review missed completed key-log values and descriptor capabilities retained by file-close errors. The latest regressions inspect the actual public object graph, including non-enumerable causes and nested cleanupPrimary, rather than relying on JSON projection.

Scope decision and limits

The requested maintainer decision covers caller-supplied session-secret key logs and their private-copy lifecycle, withheld raw producer diagnostics, removal of named credentials and URL userinfo, and restriction to the explicit metadata set with request-target context and no display filters.

Selected metadata may still contain application data or caller-embedded secrets; no arbitrary content-based DLP is claimed. Decrypted metadata can remain in MCP Evidence memory. Existing no-key-log/plaintext handling and embedded pcapng secrets need a separate policy decision.

The provider remains Linux/TShark 4.x with trusted global Wireshark configuration, plugins and libraries. No live capture, target launch, arbitrary preferences, HAR conversion or Windows admission is added. QUIC/HTTP/3, browser-produced key logs and pcapng Decryption Secrets Blocks are unverified.

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. (The existing owned TShark child, not the target.)
Inject instrumentation or run scripts/code. (Not applicable: REA launches its own TShark child, ticked above, which parses the saved capture file offline. Nothing is injected, and no target code runs.)
Change process memory, registers, files, or configuration. (Only REA's own private workspace: a read-only copy of the key log is written there and deleted after the run, matching the tool's declared writesFilesystem effect. The caller's key log and capture file are never modified.)
Contact network services.
Recover protected code or work around protections. (Not ticked: it decrypts recorded traffic only with session keys that the caller's own TLS client wrote to its key log, as Wireshark does with SSLKEYLOGFILE. No pinning, DRM or other protection is bypassed. Because this is a judgment call, "Not sure yet" is ticked and the scope question is open in #3594.)
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed. (Scope decision in #3594.)

Verification

Current code: 141 focused tests passed across 10 files: key-log lifecycle (17), artifact cleanup ownership (26), stable artifact reads (14), Linux limits (10), report/schema (18), service (18), provider output (17), command preparation (8), CLI cleanup (7), and MCP (6).
All three new source/integrity file-close regressions fail against the previous implementation with key-log bytes, readable owners and cleanup capabilities exposed. They pass with this fix, including exact source-close cancellation identity, blocked new acquisition, failed close and eventual closure of the same descriptor.
npm run check and git diff --check passed. Independent source review found no additional concrete issue in the latest fix. The published tree matches the tested tree.
Current-commit real TShark CI passed: 137 tests passed, one skipped; all 31 verifier cases passed, including all 17 key-log lifecycle cases and all 47 native capture-provider cases. Broader CI completed successfully: all 43 reported check runs on this commit passed. Greptile reviewed 22 files and added zero comments. Codex completed; the sole unresolved review thread is the maintainer scope decision. A fresh check on October 18 found no newly reported code bug.
Local real-provider/native verification remains unavailable: TShark is not installed, and this host's ps ownership probe fails with fatal library error, lookup self. The new regressions use real files, descriptor ownership and stable reads with an injected command process.
Before this internal error-privacy fix, docs/conformance and test discovery passed; all 1,582 test files were discovered exactly once. No contract, documentation or test discovery paths changed in the latest commit.

Representative normalized output from the injected-command late-finalization fixture (simulated producer data):

{"packets":[{"frame.number":["1"],"http.request.method":["GET"]}],"redactions":[{"frame":"1","field":"http.request.uri","reason":"transport-credential"},{"frame":"1","field":"http.authorization","reason":"transport-credential"},{"frame":"1","field":"http.host","reason":"transport-credential"},{"frame":"1","field":"http2.headers.authority","reason":"transport-credential"}],"diagnostics":"","withheld_diagnostic_bytes":0}

Missing decrypted fields do not establish absence; partial Evidence never asserts failed cleanup completed.

Author readiness

Human owner: @Y0UW1SH

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.

_{🔄 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/3643 **Author:** [@Y0UW1SH](https://github.com/Y0UW1SH) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `feat/3594-tls-key-log-packet-capture` --- ### 📝 Commits (10+) - [`c59840d`](https://github.com/morluto/rea/commit/c59840d1fd0e6f993a717bc805b99d6db1e7833a) feat(captures): decrypt TLS in recorded packet captures with a caller-supplied key log - [`5ee2928`](https://github.com/morluto/rea/commit/5ee2928fa429b784da6fd152334c9cdf63bca347) fix(captures): preserve TLS key-log lifecycle through upstream sync - [`cd52910`](https://github.com/morluto/rea/commit/cd5291077c51a718743c38509dbcb392f9bbac90) fix(process): retain exact kernel-limit descriptor cleanup owner - [`c2aea8f`](https://github.com/morluto/rea/commit/c2aea8f2b3aa55e563720d7118d9da05409ae046) fix(ci): resolve upstream formatting gate after sync - [`2737cf5`](https://github.com/morluto/rea/commit/2737cf5cabf7b1435141f45c9fdd5c517624338b) test(verify): isolate Ghidra prerequisite rejection from host tools - [`b34b1bf`](https://github.com/morluto/rea/commit/b34b1bfba39e82863b4ae2967e5945370387b6b9) test(apple): align archive NaN assertions with current contracts - [`1aaa504`](https://github.com/morluto/rea/commit/1aaa5044dce81b30963562461167bce9ffb84121) chore: sync main and reconcile equivalent fixture fixes - [`7a38d99`](https://github.com/morluto/rea/commit/7a38d99454e5d6eb11fd2b416eb0376b5a0b49ab) test(process): scope deadline watchdog to active execution - [`fe2d688`](https://github.com/morluto/rea/commit/fe2d688db9a593f7efda5e6ef477c890bd5ae23f) fix(captures): withhold nested decrypted command output - [`2b332cf`](https://github.com/morluto/rea/commit/2b332cfcb6313a23c7d532a3cf3efc9250a9c48e) test(cli): isolate module-mocked command finalization ### 📊 Changes **22 files changed** (+3242 additions, -102 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+7 -1) 📝 `.github/workflows/real-packet-capture.yml` (+3 -1) 📝 `docs/packet-captures.md` (+112 -0) ➕ `scripts/fixtures/tls-key-log-capture.mjs` (+204 -0) 📝 `scripts/verify-packet-capture.mjs` (+250 -18) ➕ `src/application/captures/PacketCaptureService.test.ts` (+180 -0) 📝 `src/application/captures/PacketCaptureService.ts` (+17 -9) 📝 `src/captures/TsharkCaptureProvider.ts` (+231 -44) 📝 `src/cli/captureCommands.ts` (+7 -0) 📝 `src/contracts/captures/captureToolContracts.ts` (+9 -1) ➕ `src/domain/captures/packetCapture.test.ts` (+315 -0) 📝 `src/domain/captures/packetCapture.ts` (+263 -8) ➕ `src/domain/captures/tlsKeyLog.test.ts` (+188 -0) ➕ `src/domain/captures/tlsKeyLog.ts` (+210 -0) 📝 `src/process/ArtifactAnalysisResources.test.ts` (+49 -0) 📝 `src/process/ArtifactAnalysisResources.ts` (+47 -9) 📝 `src/process/capture/ProcessCaptureLifecycle.test.ts` (+27 -1) 📝 `tests/boundary/captures/captureProvider.test.ts` (+325 -3) ➕ `tests/boundary/captures/tsharkKeyLogLifecycle.test.ts` (+664 -0) 📝 `tests/boundary/cli/captureCleanup.test.ts` (+46 -0) _...and 2 more files_ </details> ### 📄 Description ## Summary Recorded HTTPS inspection needs matching client session secrets to return decrypted fields. Add optional `tls_key_log_path` for a key log produced by the caller's own client. TShark receives a private read-only copy; REA returns its identity/counts and selected reviewed metadata, removes transport credentials and URL userinfo, and withholds raw producer diagnostics. **Current head: [`6fd11bd`](https://github.com/morluto/rea/commit/6fd11bdd0258b5c6e938ba863ceef6bdb3111308). The latest file-close error leak is fixed and pushed; 141 focused tests and static checks pass locally. All 43 reported checks passed. Automated reviews completed; the sole unresolved review thread is the maintainer scope decision.** Merging remains blocked by the explicit [maintainer scope decision in #3594](https://github.com/morluto/rea/issues/3594#issuecomment-6243385786) required by the [changes-requested review](https://github.com/morluto/rea/pull/3643#pullrequestreview-5528977451). Review readiness and CI success do not supply scope approval. Refs #3594. Related TShark selection work: #3403 / #3407. `inspect_pcap_capture` is unchanged. ## Implementation and follow-up fixes - Read the caller's key log through the stable artifact reader, bounded to 32 MiB. Report identity, line/label counts and distinct client-random counts without secrets, line text or random values. Counts do not prove decryption; unrecognized lines remain available to TShark. - Pass a private mode-`0400` copy as one `tls.keylog_file` argument, verify private input integrity after execution, and attempt prompt deletion. Retain exact cleanup ownership across failures. Completed redacted Evidence survives late cancellation and workspace cleanup failure. - Admit only the guide's explicit 30 packet/HTTP metadata fields for key-log requests. Named authorization/cookie fields, including supported `@` variants, are admitted only for removal. Refuse generic headers, decoded authentication, payload/raw selections, unreviewed fields and every non-null display filter before provider work. - Remove whole selected URI/authority fields containing structural userinfo and record removal. Require selected request methods with HTTP request URIs; refuse uncertain counts and mixed CONNECT/other contexts. Preserve origin-form paths, query data and opaque URI data without guessing secrecy. Validate returned reports against the same boundary. - Withhold decrypting stdout/stderr throughout actual errors, including nested command wrappers. Preserve sizes, exact cancellation reason and physical process facts; keep cleanup supervisors private. - **Latest fix:** both key-log reads mark their exact failure and cleanup leases confidential. Public cleanup errors omit raw wrappers that retain completed key-log bytes and readable file handles. The same private leases retain cleanup authority through failed admission retries and `close()`; successful retry clears the original errors' pending state. Ordinary artifact reads retain their existing behavior. - The portable WASM ordering failure reproduced on clean upstream `847472a`; upstream independently fixed it in `8cbaf6c`. This branch includes the exact upstream isolated test through the sync with main `bb212ff`, retaining all five assertions. - Contracts, CLI/MCP coverage, guide and shipped skill describe the narrowed behavior. The real verifier includes all selected frames, a credential-bearing absolute URI, generic-field/filter refusals and an admitted producer-failure request. The previous review missed completed key-log values and descriptor capabilities retained by file-close errors. The latest regressions inspect the actual public object graph, including non-enumerable causes and nested `cleanupPrimary`, rather than relying on JSON projection. ## Scope decision and limits The requested maintainer decision covers caller-supplied session-secret key logs and their private-copy lifecycle, withheld raw producer diagnostics, removal of named credentials and URL userinfo, and restriction to the explicit metadata set with request-target context and no display filters. Selected metadata may still contain application data or caller-embedded secrets; no arbitrary content-based DLP is claimed. Decrypted metadata can remain in MCP Evidence memory. Existing no-key-log/plaintext handling and embedded pcapng secrets need a separate policy decision. The provider remains Linux/TShark 4.x with trusted global Wireshark configuration, plugins and libraries. No live capture, target launch, arbitrary preferences, HAR conversion or Windows admission is added. QUIC/HTTP/3, browser-produced key logs and pcapng Decryption Secrets Blocks are unverified. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [x] Launch or attach to a process. (The existing owned TShark child, not the target.) - [ ] Inject instrumentation or run scripts/code. (Not applicable: REA launches its own TShark child, ticked above, which parses the saved capture file offline. Nothing is injected, and no target code runs.) - [x] Change process memory, registers, files, or configuration. (Only REA's own private workspace: a read-only copy of the key log is written there and deleted after the run, matching the tool's declared `writesFilesystem` effect. The caller's key log and capture file are never modified.) - [ ] Contact network services. - [ ] Recover protected code or work around protections. (Not ticked: it decrypts recorded traffic only with session keys that the caller's own TLS client wrote to its key log, as Wireshark does with `SSLKEYLOGFILE`. No pinning, DRM or other protection is bypassed. Because this is a judgment call, "Not sure yet" is ticked and the scope question is open in #3594.) - [ ] Documentation or usability only; no target interaction. - [x] Not sure yet; investigation or maintainer guidance needed. (Scope decision in #3594.) ## Verification - **Current code: 141 focused tests passed across 10 files**: key-log lifecycle (17), artifact cleanup ownership (26), stable artifact reads (14), Linux limits (10), report/schema (18), service (18), provider output (17), command preparation (8), CLI cleanup (7), and MCP (6). - All three new source/integrity file-close regressions fail against the previous implementation with key-log bytes, readable owners and cleanup capabilities exposed. They pass with this fix, including exact source-close cancellation identity, blocked new acquisition, failed close and eventual closure of the same descriptor. - `npm run check` and `git diff --check` passed. Independent source review found no additional concrete issue in the latest fix. The published tree matches the tested tree. - **Current-commit real TShark CI passed:** [137 tests passed, one skipped; all 31 verifier cases passed](https://github.com/morluto/rea/actions/runs/39100664353/job/117596093684), including all 17 key-log lifecycle cases and all 47 native capture-provider cases. [Broader CI](https://github.com/morluto/rea/actions/runs/39100664437) completed successfully: all 43 reported check runs on this commit passed. Greptile reviewed 22 files and added zero comments. Codex completed; the sole unresolved review thread is the maintainer scope decision. A fresh check on October 18 found no newly reported code bug. - Local real-provider/native verification remains unavailable: TShark is not installed, and this host's `ps` ownership probe fails with `fatal library error, lookup self`. The new regressions use real files, descriptor ownership and stable reads with an injected command process. - Before this internal error-privacy fix, docs/conformance and test discovery passed; all 1,582 test files were discovered exactly once. No contract, documentation or test discovery paths changed in the latest commit. Representative normalized output from the injected-command late-finalization fixture (simulated producer data): ```json {"packets":[{"frame.number":["1"],"http.request.method":["GET"]}],"redactions":[{"frame":"1","field":"http.request.uri","reason":"transport-credential"},{"frame":"1","field":"http.authorization","reason":"transport-credential"},{"frame":"1","field":"http.host","reason":"transport-credential"},{"frame":"1","field":"http2.headers.authority","reason":"transport-credential"}],"diagnostics":"","withheld_diagnostic_bytes":0} ``` Missing decrypted fields do not establish absence; partial Evidence never asserts failed cleanup completed. ## Author readiness  Human owner: @Y0UW1SH - [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 - [x] 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. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:00 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:19 +00:00

[PR #5862] [MERGED] fix(android): report a rejected relative JADX JAR instead of an unset one #7091

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#5862

No description provided.

Rows
Columns