morluto/rea

Fork 0

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

[PR #2695] [CLOSED] fix(ida): retain upstream tool error diagnostics #5320

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2695
Author: @g1at
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: codex/ida-upstream-error-diagnostics

📝 Commits (10+)

ecee301 fix(ida): retain upstream tool error diagnostics
5c1a30e fix(ida): redact credentials from tool diagnostics
7d79125 fix(ida): parse configured authentication payloads
a0a6d64 fix(ida): redact JSON diagnostics by decoded source spans
4a8c751 fix(ida): unify malformed JSON diagnostic recovery
8524361 fix(ida): retain raw credentials in malformed JSON diagnostics
6d9c1d3 fix(ida): bound and sanitize transport diagnostics at their owner
46cd1d7 fix(ida): preserve safe diagnostic causes and bounded matching
08deea0 fix(ida): apply encoding policy to malformed HTTP JSON
97c68b8 fix(ida): redact canonical Basic credential spellings

📊 Changes

12 files changed (+3409 additions, -89 deletions)

View changed files

➕ src/ida/IdaDiagnosticRedactor.test.ts (+563 -0)
➕ src/ida/IdaDiagnosticRedactor.ts (+243 -0)
➕ src/ida/IdaDiagnosticWriter.ts (+85 -0)
➕ src/ida/IdaJsonDiagnostic.test.ts (+337 -0)
➕ src/ida/IdaJsonDiagnostic.ts (+216 -0)
📝 src/ida/IdaMcpConnection.test.ts (+263 -21)
📝 src/ida/IdaMcpConnection.ts (+211 -34)
📝 src/ida/IdaMcpEnvironment.test.ts (+12 -1)
📝 src/ida/IdaOperationFailures.test.ts (+191 -1)
📝 src/ida/IdaSessionClient.test.ts (+26 -3)
📝 tests/boundary/idaStartupDiagnostics.test.ts (+12 -4)
📝 tests/boundary/mcp/idaTransport.test.ts (+1250 -25)

📄 Description

Problem and outcome

IDA MCP tool errors could lose the producer's explanation or retain configured authentication inside diagnostics and nested transport errors. This change exposes useful upstream reasons in existing CLI/MCP error diagnostics while keeping credential handling at the IDA boundary. For example, an upstream invalid-parameter explanation now remains available in details.diagnostics.reason.

Refs #2301; complements #2307 and #2449. It does not add support for an otherwise incompatible upstream protocol.

Behavior and ownership

Configured Authorization, Proxy-Authorization, and Cookie values have one interpretation owner. Basic protects complete decoded pairs, nonempty passwords, and supported Base64 spellings; standalone usernames remain identity values. Unknown Basic charset retains strict UTF-8 and exact Latin-1 candidates, without lossy decoding, BOM removal, or URL decoding.
Diagnostic inputs have the maintained 64-KiB UTF-8 source budget, including configurations without credentials. Oversized text produces an explicit omission notice before JSON interpretation or matching. Successful observations are outside this budget.
JSON redaction preserves source spelling, duplicate keys, exact numeric lexemes, and unrelated caller context. Only a strictly valid whole document permits decoded-only string interpretation. Malformed documents retain both raw and complete decoded-run matches. Overlapping occurrences are covered and their source spans are merged before buffered output; invalid units do not join fragments. The standard replacement label is retained only when it cannot contain or join a credential; otherwise a graphic Unicode scalar absent from every credential is selected. This also prevents marker boundaries from creating a new credential.
One complete Content-Type parser owns format and charset decisions for non-2xx responses and successful-status JSON parse failures. It consumes the entire field, including HTTP quoted pairs, and treats malformed or duplicate parameters as unknown rather than silently discarding them. Supported JSON/+json and plain text retain sanitized diagnostics; other formats or unsupported/uncertain encodings explicitly omit the body.
Only the pinned SDK producer's exact POST-body position is trusted as already sanitized. Other SDK context is redacted independently, with LF separating owners. Retained causes contain newly constructed safe messages and known HTTP status/protocol code; raw bodies, parser exceptions, arbitrary SDK data, old stacks, and nested error graphs are excluded. Owned capacity failures retain their typed public projection.

No public operation, schema, target action, or authority is added. Successfully parsed results, lazy successful JSON, SSE streams, and caller-selected paths/URLs retain their existing contracts. Windows fixture corrections explicitly select the Node launcher's actual SYSTEMROOT and release only registered test-owned native runtimes after retention assertions; production resource retention is unchanged.

Validation

Final candidate: 09283a166f.

Focused regression result and elapsed time: 21 files: 416 passed, 7 existing platform-conditioned skips; 23.3 seconds..
Typecheck, format, lint, knip, metadata, and architecture checks: Passed npm run check (20.8 seconds): 0 lint errors, 1090 warnings; all listed checks and module guards passed..
Final Windows IDA CLI/MCP and installed Relay bridge result: Passed public CLI/MCP IDA verification (15.9 seconds), including feature tracing, distinct-function switching, invalid input, unchanged fixture and verified owned cleanup. Installed Relay 3.0.2 bridge passed (7.4 seconds), with session/bridge close and unchanged registry entry..
Automatic review state at the final commit: Awaiting both automatic reviews of this exact commit..

Independent production parser cases passed 60/60 and marker-boundary cases passed 24/24. Two line-wrap-only fixes from the merged main were required by the full format gate.

Completed supporting checks: the three Windows fixture files passed 63/63 tests in 17.06s, including their required runtime build. The native addon was rebuilt with the repository's standard MinGW lane against the merged sources; source hashes, artifact SHA/PE checks, and Windows Node loading passed. Its old artifact was backed up, and the arithmetic ELF fixture digest remained unchanged.

The real-provider lane uses Windows IDA Pro 9.2 / idalib / Hex-Rays with compatible ida-pro-mcp and Relay's actual installed stdio bridge, through a private candidate launch override. It analyzes a self-compiled arithmetic ELF statically, without executing it, and closes only owned headless sessions without saving a database. Fixture SHA-256: b20b67bac6830e5e7aaf36478b952f54703afd6647319a17268d7d0cd09467d3.

Authentication, malformed-header/body, overlap, replacement-collision, and error-projection cases use the pinned SDK and local synthetic producers; they do not establish authenticated real IDA HTTP behavior. The Windows lifecycle fixtures use a recording producer and real filesystem ownership, not an IDA worker.

Limits

Attached GUI behavior, authenticated IDA HTTP, arbitrary historical Basic charsets, and future authProvider/DPoP or response-clone consumers remain unverified. Real xref correctness beyond contract checks lacks an independently supplied expected address. Other platform/provider coverage and the full repository suite: not rerun; the seven skipped ownership/thinning cases require another platform..

Seven bounded helper microbenchmark samples passed independent output oracles. For equivalent 65,536-byte source tasks, complete output hashes matched revision 8524361; two fresh-child samples measured old 35.2/36.3 ms and final 31.7/30.8 ms, old total CPU 46/62 ms and final 78/47 ms, and peak single-process RSS 103,964/104,520 KiB versus 71,612/71,232 KiB. These are descriptive samples, with initialization/marker selection excluded and concurrent checks possible; no stable speed or CPU improvement is claimed. Cold-only helper measurements do not measure agent tokens, warm tasks, completed CLI/MCP task CPU, or process-family peak memory. Inputs beyond the source budget now produce omission, so their outputs are not equivalent redaction workflows. Binaries, databases, credentials, ambient environment snapshots, and private verification reports are excluded from commits.

_{🔄 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/2695 **Author:** [@g1at](https://github.com/g1at) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/ida-upstream-error-diagnostics` --- ### 📝 Commits (10+) - [`ecee301`](https://github.com/morluto/rea/commit/ecee3013f09edf3e141f2b2959f7b5d5ee2db665) fix(ida): retain upstream tool error diagnostics - [`5c1a30e`](https://github.com/morluto/rea/commit/5c1a30e9d387985f3fb2be8337ab758d54f2f7de) fix(ida): redact credentials from tool diagnostics - [`7d79125`](https://github.com/morluto/rea/commit/7d79125b6262ea8608bfc7ca22715af7d9283e61) fix(ida): parse configured authentication payloads - [`a0a6d64`](https://github.com/morluto/rea/commit/a0a6d6422e3f5945d3822e25239f039cc2eb1522) fix(ida): redact JSON diagnostics by decoded source spans - [`4a8c751`](https://github.com/morluto/rea/commit/4a8c751a896458a1aedf4a2513b5d589d096dfeb) fix(ida): unify malformed JSON diagnostic recovery - [`8524361`](https://github.com/morluto/rea/commit/8524361bb1370f2c32ad0798882a483e56f3f358) fix(ida): retain raw credentials in malformed JSON diagnostics - [`6d9c1d3`](https://github.com/morluto/rea/commit/6d9c1d3be50c4d3618a0d5d1ee200bf49fa808e8) fix(ida): bound and sanitize transport diagnostics at their owner - [`46cd1d7`](https://github.com/morluto/rea/commit/46cd1d7ae4ab562576720b661c3768e85a7fe2c4) fix(ida): preserve safe diagnostic causes and bounded matching - [`08deea0`](https://github.com/morluto/rea/commit/08deea0592ea083afdb1e5b0f7db062cdb17bf8e) fix(ida): apply encoding policy to malformed HTTP JSON - [`97c68b8`](https://github.com/morluto/rea/commit/97c68b86444574eef7142ead131c97d6beca136c) fix(ida): redact canonical Basic credential spellings ### 📊 Changes **12 files changed** (+3409 additions, -89 deletions) <details> <summary>View changed files</summary> ➕ `src/ida/IdaDiagnosticRedactor.test.ts` (+563 -0) ➕ `src/ida/IdaDiagnosticRedactor.ts` (+243 -0) ➕ `src/ida/IdaDiagnosticWriter.ts` (+85 -0) ➕ `src/ida/IdaJsonDiagnostic.test.ts` (+337 -0) ➕ `src/ida/IdaJsonDiagnostic.ts` (+216 -0) 📝 `src/ida/IdaMcpConnection.test.ts` (+263 -21) 📝 `src/ida/IdaMcpConnection.ts` (+211 -34) 📝 `src/ida/IdaMcpEnvironment.test.ts` (+12 -1) 📝 `src/ida/IdaOperationFailures.test.ts` (+191 -1) 📝 `src/ida/IdaSessionClient.test.ts` (+26 -3) 📝 `tests/boundary/idaStartupDiagnostics.test.ts` (+12 -4) 📝 `tests/boundary/mcp/idaTransport.test.ts` (+1250 -25) </details> ### 📄 Description ## Problem and outcome IDA MCP tool errors could lose the producer's explanation or retain configured authentication inside diagnostics and nested transport errors. This change exposes useful upstream reasons in existing CLI/MCP error diagnostics while keeping credential handling at the IDA boundary. For example, an upstream invalid-parameter explanation now remains available in `details.diagnostics.reason`. Refs [#2301](https://github.com/morluto/rea/issues/2301); complements [#2307](https://github.com/morluto/rea/pull/2307) and [#2449](https://github.com/morluto/rea/pull/2449). It does not add support for an otherwise incompatible upstream protocol. ## Behavior and ownership - Configured Authorization, Proxy-Authorization, and Cookie values have one interpretation owner. Basic protects complete decoded pairs, nonempty passwords, and supported Base64 spellings; standalone usernames remain identity values. Unknown Basic charset retains strict UTF-8 and exact Latin-1 candidates, without lossy decoding, BOM removal, or URL decoding. - Diagnostic inputs have the maintained 64-KiB UTF-8 source budget, including configurations without credentials. Oversized text produces an explicit omission notice before JSON interpretation or matching. Successful observations are outside this budget. - JSON redaction preserves source spelling, duplicate keys, exact numeric lexemes, and unrelated caller context. Only a strictly valid whole document permits decoded-only string interpretation. Malformed documents retain both raw and complete decoded-run matches. Overlapping occurrences are covered and their source spans are merged before buffered output; invalid units do not join fragments. The standard replacement label is retained only when it cannot contain or join a credential; otherwise a graphic Unicode scalar absent from every credential is selected. This also prevents marker boundaries from creating a new credential. - One complete Content-Type parser owns format and charset decisions for non-2xx responses and successful-status JSON parse failures. It consumes the entire field, including HTTP quoted pairs, and treats malformed or duplicate parameters as unknown rather than silently discarding them. Supported JSON/+json and plain text retain sanitized diagnostics; other formats or unsupported/uncertain encodings explicitly omit the body. - Only the pinned SDK producer's exact POST-body position is trusted as already sanitized. Other SDK context is redacted independently, with LF separating owners. Retained causes contain newly constructed safe messages and known HTTP status/protocol code; raw bodies, parser exceptions, arbitrary SDK data, old stacks, and nested error graphs are excluded. Owned capacity failures retain their typed public projection. No public operation, schema, target action, or authority is added. Successfully parsed results, lazy successful JSON, SSE streams, and caller-selected paths/URLs retain their existing contracts. Windows fixture corrections explicitly select the Node launcher's actual SYSTEMROOT and release only registered test-owned native runtimes after retention assertions; production resource retention is unchanged. ## Validation Final candidate: **09283a166ffea488c59e2b0d5ac3c4cd31a4007a**. - Focused regression result and elapsed time: **21 files: 416 passed, 7 existing platform-conditioned skips; 23.3 seconds.**. - Typecheck, format, lint, knip, metadata, and architecture checks: **Passed npm run check (20.8 seconds): 0 lint errors, 1090 warnings; all listed checks and module guards passed.**. - Final Windows IDA CLI/MCP and installed Relay bridge result: **Passed public CLI/MCP IDA verification (15.9 seconds), including feature tracing, distinct-function switching, invalid input, unchanged fixture and verified owned cleanup. Installed Relay 3.0.2 bridge passed (7.4 seconds), with session/bridge close and unchanged registry entry.**. - Automatic review state at the final commit: **Awaiting both automatic reviews of this exact commit.**. Independent production parser cases passed 60/60 and marker-boundary cases passed 24/24. Two line-wrap-only fixes from the merged main were required by the full format gate. Completed supporting checks: the three Windows fixture files passed 63/63 tests in 17.06s, including their required runtime build. The native addon was rebuilt with the repository's standard MinGW lane against the merged sources; source hashes, artifact SHA/PE checks, and Windows Node loading passed. Its old artifact was backed up, and the arithmetic ELF fixture digest remained unchanged. The real-provider lane uses Windows IDA Pro 9.2 / idalib / Hex-Rays with compatible ida-pro-mcp and Relay's actual installed stdio bridge, through a private candidate launch override. It analyzes a self-compiled arithmetic ELF statically, without executing it, and closes only owned headless sessions without saving a database. Fixture SHA-256: `b20b67bac6830e5e7aaf36478b952f54703afd6647319a17268d7d0cd09467d3`. Authentication, malformed-header/body, overlap, replacement-collision, and error-projection cases use the pinned SDK and local synthetic producers; they do not establish authenticated real IDA HTTP behavior. The Windows lifecycle fixtures use a recording producer and real filesystem ownership, not an IDA worker. ## Limits Attached GUI behavior, authenticated IDA HTTP, arbitrary historical Basic charsets, and future authProvider/DPoP or response-clone consumers remain unverified. Real xref correctness beyond contract checks lacks an independently supplied expected address. Other platform/provider coverage and the full repository suite: **not rerun; the seven skipped ownership/thinning cases require another platform.**. Seven bounded helper microbenchmark samples passed independent output oracles. For equivalent 65,536-byte source tasks, complete output hashes matched revision 8524361; two fresh-child samples measured old 35.2/36.3 ms and final 31.7/30.8 ms, old total CPU 46/62 ms and final 78/47 ms, and peak single-process RSS 103,964/104,520 KiB versus 71,612/71,232 KiB. These are descriptive samples, with initialization/marker selection excluded and concurrent checks possible; no stable speed or CPU improvement is claimed. Cold-only helper measurements do not measure agent tokens, warm tasks, completed CLI/MCP task CPU, or process-family peak memory. Inputs beyond the source budget now produce omission, so their outputs are not equivalent redaction workflows. Binaries, databases, credentials, ambient environment snapshots, and private verification reports are excluded from commits. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5321] [CLOSED] fix(artifacts): note legacy code page readings of unflagged ZIP names #6878

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

No description provided.

Rows
Columns