morluto/rea

Fork 0

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

[PR #2872] [CLOSED] fix(captures): redact normalized HAR numeric values #5458

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/har-normalized-number-redaction

📝 Commits (2)

1cf3d0a fix(captures): redact normalized HAR numeric values
889c43a Merge remote-tracking branch 'origin/main' into sweep/2872

📊 Changes

1 file changed (+5 additions, -9 deletions)

View changed files

📝 src/browser/history/QlogCapture.ts (+5 -9)

📄 Description

Summary

Fix HAR numeric redaction when JSON normalization introduces a caller-excluded literal. With _number: 1e3 and sensitive_values: ["1000"], inspection previously returned _number: 1000. It now returns null, omits that numeric sidecar, and records an explicit-sensitive-value redaction. Unmarked values and exact producer lexemes remain retained.

Problem and expected behavior

HAR checked declarations against the original number lexeme, but omitted the normalized spelling it actually published. Both 1e3 → 1000 and 0.0000001 → 1e-7 could reintroduce excluded text. Exclusion must apply before reported values or numeric sidecars are retained, in container metadata and extracted records.

Change and scope

CaptureRedaction.number owns safe conversion and checks for both spellings. HAR and qlog use this shared owner; qlog already checked normalized values, so the refactor preserves its behavior and removes duplicated interpretation. HAR schema admission still validates the unredacted projection. No downstream filter or directly related cleanup remains.

This is an ordinary fix within existing offline capture inspection; no advance maintainer scope decision applies. No separate issue is linked because the bug was found and reproduced during review. Related-PR searches found no equivalent fix; #2477 addresses binary byte counts.

Contract and boundary impact

Semantic owner: capture numeric projection before publishing values and sidecars.
CLI/MCP schemas, dependencies, generated metadata and installation behavior: unchanged.
Evidence: existing null values and redaction records express the corrected exclusion; unmarked lexemes and artifact provenance remain retained.
Authority: existing offline reads only; no new target execution, instrumentation, mutation, network acquisition or protection recovery.

Evidence and regression coverage

Source regressions cover container metadata and records, sidecar omission, exact redaction pointers and unmarked controls. Compiled CLI/stdio MCP regressions also verify matching Evidence identities and normalized results, plus unchanged input files.

On clean base 117bd23dd, both new end-to-end cases fail after successful CLI/MCP decoding: excluded values remain 1000 and 1e-7. Source regressions reproduce the same failures. On the fixed tree, all 107 related tests across six files pass, with no skips.

Validation performed

Windows x64, pinned Node 24.18.0, npm 11.16.0, locked dependencies installed with npm ci --cache .cache/npm --foreground-scripts; installation scripts completed normally. Fixed and baseline runtimes were freshly compiled with npm run build. End-to-end runs used the same Windows user permissions, toolchain, dependencies and native addon. Earlier sandbox access-denial attempts are not counted as passing verification.

The native addon comes from upstream CI run 38395741217, commit 1729e87d9f2d1e9db797864cdfe763a327e13049. All four native source blobs and the build script match this checkout; both package versions are 6.4.0. Archive SHA-256 matches GitHub's digest. The current artifact verifier confirms addon SHA-256, package version, Windows x64/PE32+, Node-API 8 and ABI 1. Generated artifacts and the local toolchain are ignored and uncommitted.

npm run test:focused -- src/browser/history/HarCapture.test.ts src/browser/history/QlogCapture.test.ts src/application/WebNetworkCaptureService.test.ts tests/boundary/browser/historicalWebCapture.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts tests/acceptance/tools/historicalCaptureResources.test.ts — 107 passed, including real HAR memory exhaustion and server usability afterward.
npm run build, npm run verify:windows-native:artifact — passed; builds report two ineffective dynamic-import warnings.
npm run typecheck, npm run knip, npm run verify:module-boundaries, npm run verify:architecture-guards, npm run verify:test-discovery — passed; all 1,071 test files discovered once.
Changed-file formatting/lint, normal pre-commit hooks and git diff --check — passed; lint retains two existing function-length warnings.
npm run check — fails formatting in unchanged tests/fixtures/javaClassFile.ts.
npm run check:fast — fails function-length lint in unchanged tests/boundary/providers/ghidra/ghidraLauncher.test.ts:431 and src/ghidra/GhidraFunctionValues.test.ts:46.

Both aggregate-check failures were independently reproduced with the same commands on clean base 117bd23dd under the pinned toolchain. They remain unresolved and are not reported as passing checks.

Compatibility, safety, and release

No schema changes, migration, dependency updates or release metadata changes. Real Windows CLI/MCP capture-worker behavior is verified with synthetic offline HAR files. Real browser, Hopper, Ghidra and mitmproxy producer execution, Linux/macOS runs and a full-suite pass are not claimed.

Before merge

Bug reproduced on the base and corrected through public CLI/MCP workflows.
Existing contribution scope and authority boundaries preserved.
Focused outcome, Conventional Commit title and regression coverage.
Final diff checked for unrelated changes and generated artifacts.
Aggregate static checks and current-head CI pass; baseline failures are recorded above.

_{🔄 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/2872 **Author:** [@lzhMeow](https://github.com/lzhMeow) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/har-normalized-number-redaction` --- ### 📝 Commits (2) - [`1cf3d0a`](https://github.com/morluto/rea/commit/1cf3d0a54db1ca5606200000bf792e8c66521356) fix(captures): redact normalized HAR numeric values - [`889c43a`](https://github.com/morluto/rea/commit/889c43ace8194da365e8581d8072ac54f50e0f42) Merge remote-tracking branch 'origin/main' into sweep/2872 ### 📊 Changes **1 file changed** (+5 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/history/QlogCapture.ts` (+5 -9) </details> ### 📄 Description ## Summary Fix HAR numeric redaction when JSON normalization introduces a caller-excluded literal. With `_number: 1e3` and `sensitive_values: ["1000"]`, inspection previously returned `_number: 1000`. It now returns null, omits that numeric sidecar, and records an `explicit-sensitive-value` redaction. Unmarked values and exact producer lexemes remain retained. ## Problem and expected behavior HAR checked declarations against the original number lexeme, but omitted the normalized spelling it actually published. Both `1e3` → `1000` and `0.0000001` → `1e-7` could reintroduce excluded text. Exclusion must apply before reported values or numeric sidecars are retained, in container metadata and extracted records. ## Change and scope `CaptureRedaction.number` owns safe conversion and checks for both spellings. HAR and qlog use this shared owner; qlog already checked normalized values, so the refactor preserves its behavior and removes duplicated interpretation. HAR schema admission still validates the unredacted projection. No downstream filter or directly related cleanup remains. This is an ordinary fix within existing offline capture inspection; no advance maintainer scope decision applies. No separate issue is linked because the bug was found and reproduced during review. Related-PR searches found no equivalent fix; #2477 addresses binary byte counts. ## Contract and boundary impact - Semantic owner: capture numeric projection before publishing values and sidecars. - CLI/MCP schemas, dependencies, generated metadata and installation behavior: unchanged. - Evidence: existing null values and redaction records express the corrected exclusion; unmarked lexemes and artifact provenance remain retained. - Authority: existing offline reads only; no new target execution, instrumentation, mutation, network acquisition or protection recovery. ## Evidence and regression coverage Source regressions cover container metadata and records, sidecar omission, exact redaction pointers and unmarked controls. Compiled CLI/stdio MCP regressions also verify matching Evidence identities and normalized results, plus unchanged input files. On clean base `117bd23dd`, both new end-to-end cases fail after successful CLI/MCP decoding: excluded values remain `1000` and `1e-7`. Source regressions reproduce the same failures. On the fixed tree, all 107 related tests across six files pass, with no skips. ## Validation performed Windows x64, pinned Node **24.18.0**, npm **11.16.0**, locked dependencies installed with `npm ci --cache .cache/npm --foreground-scripts`; installation scripts completed normally. Fixed and baseline runtimes were freshly compiled with `npm run build`. End-to-end runs used the same Windows user permissions, toolchain, dependencies and native addon. Earlier sandbox access-denial attempts are not counted as passing verification. The native addon comes from upstream [CI run 38395741217](https://github.com/morluto/rea/actions/runs/38395741217), commit `1729e87d9f2d1e9db797864cdfe763a327e13049`. All four native source blobs and the build script match this checkout; both package versions are 6.4.0. Archive SHA-256 matches GitHub's digest. The current artifact verifier confirms addon SHA-256, package version, Windows x64/PE32+, Node-API 8 and ABI 1. Generated artifacts and the local toolchain are ignored and uncommitted. - `npm run test:focused -- src/browser/history/HarCapture.test.ts src/browser/history/QlogCapture.test.ts src/application/WebNetworkCaptureService.test.ts tests/boundary/browser/historicalWebCapture.test.ts tests/boundary/mcp/webNetworkCaptureMcp.test.ts tests/acceptance/tools/historicalCaptureResources.test.ts` — **107 passed**, including real HAR memory exhaustion and server usability afterward. - `npm run build`, `npm run verify:windows-native:artifact` — passed; builds report two ineffective dynamic-import warnings. - `npm run typecheck`, `npm run knip`, `npm run verify:module-boundaries`, `npm run verify:architecture-guards`, `npm run verify:test-discovery` — passed; all 1,071 test files discovered once. - Changed-file formatting/lint, normal pre-commit hooks and `git diff --check` — passed; lint retains two existing function-length warnings. - `npm run check` — fails formatting in unchanged `tests/fixtures/javaClassFile.ts`. - `npm run check:fast` — fails function-length lint in unchanged `tests/boundary/providers/ghidra/ghidraLauncher.test.ts:431` and `src/ghidra/GhidraFunctionValues.test.ts:46`. Both aggregate-check failures were independently reproduced with the same commands on clean base `117bd23dd` under the pinned toolchain. They remain unresolved and are not reported as passing checks. ## Compatibility, safety, and release No schema changes, migration, dependency updates or release metadata changes. Real Windows CLI/MCP capture-worker behavior is verified with synthetic offline HAR files. Real browser, Hopper, Ghidra and mitmproxy producer execution, Linux/macOS runs and a full-suite pass are not claimed. ## Before merge - [x] Bug reproduced on the base and corrected through public CLI/MCP workflows. - [x] Existing contribution scope and authority boundaries preserved. - [x] Focused outcome, Conventional Commit title and regression coverage. - [x] Final diff checked for unrelated changes and generated artifacts. - [ ] Aggregate static checks and current-head CI pass; baseline failures are recorded above. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:27 +00:00

[PR #5581] [MERGED] fix(environment): report later bundle write failures as host failures #6978

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

No description provided.

Rows
Columns