[PR #1867] [MERGED] fix: order remaining outputs independently of host locale #4863

Closed
opened 2026-10-31 00:56:09 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1867
Author: @dollarsmoney
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @morluto

Base: main ← Head: fix/locale-independent-ordering


📝 Commits (2)

  • e16dfc7 fix: order remaining outputs independently of host locale
  • de51642 fix(artifacts): order ASAR entries by their portable path

📊 Changes

32 files changed (+174 additions, -56 deletions)

View changed files

📝 src/application/CapabilityInventory.ts (+2 -1)
📝 src/application/ClientConfigurationDocument.ts (+2 -1)
📝 src/application/ClientRegistrationEnvironment.ts (+2 -1)
📝 src/application/ClientRegistrationStatus.ts (+2 -1)
📝 src/application/ReconstructionObligationCandidates.ts (+3 -2)
📝 src/application/ReconstructionObligationLedgerDiagnostics.ts (+3 -1)
📝 src/application/ReconstructionObligationLedgerEvaluation.ts (+2 -1)
📝 src/application/ReconstructionObligationOriginalCases.ts (+3 -1)
📝 src/application/SetupPlan.ts (+2 -1)
📝 src/application/SetupRegistrationEnvironment.ts (+2 -1)
📝 src/application/SupportedClients.ts (+2 -1)
📝 src/application/binary/AnalysisProviderRegistry.ts (+5 -2)
📝 src/application/binary/BinarySessionStatus.ts (+11 -4)
📝 src/application/binary/CompositeProvider.ts (+5 -4)
📝 src/application/investigation/EvidenceLedger.ts (+4 -1)
➕ src/artifacts/AsarArtifactReader.test.ts (+25 -0)
📝 src/artifacts/AsarArtifactReader.ts (+28 -12)
📝 src/browser/CdpBrowserProvider.ts (+4 -1)
📝 src/browser/CdpCaptureDocuments.ts (+2 -1)
📝 src/browser/CdpElectronInspection.ts (+2 -1)

...and 12 more files

📄 Description

Summary

String.prototype.localeCompare() without a pinned collation follows the host's ICU default locale, which comes from LANG/LC_ALL. The same input can therefore be ordered differently on different machines. 614fb768 ("fix(evidence): order canonical projections independently of locale") moved the canonical domain projections to compareUnicodeCodePoints. Forty-two localeCompare calls in 30 provider, application, catalog and setup files still used locale collation. This PR orders all of them by Unicode code point, so no localeCompare remains in non-test source.

No linked issue: I found this while reading the code, as a direct follow-up to 614fb768.

Problem and expected behavior

Smallest trigger, on unmodified main: inventoryFirmwareOutput sorts extracted firmware paths with localeCompare.

LC_ALL=en_US.UTF-8   a.bin ä.bin B.bin z.bin
LC_ALL=sv_SE.UTF-8   a.bin B.bin z.bin ä.bin
code point           B.bin a.bin z.bin ä.bin

The violated invariant: evidence and listings derived from the same target should not depend on the analyst's locale. The same kind of drift affects Mach-O symbol lists (_main / _Zeta / Zeta interleave differently from code-point order), and any sorted set that later feeds a digest or comparison.

Change and scope

Every remaining non-test a.localeCompare(b) becomes compareUnicodeCodePoints(a, b), the existing owner in src/domain/unicodeCodePointOrder.ts. I applied it with a parser-based rewrite and reviewed the result by hand:

  • Evidence and provider output: Mach-O symbols (NativeMachoInspection), firmware extraction paths (FirmwareFiles), ASAR entries (AsarArtifactReader, previously pinned to "en" collation, which still varies by ICU version and is not code-point order), and evidence ID sets (registerInvestigationTools/helpers).
  • Browser and Electron inventories: page capture scripts, capture documents, targets, workers, scripts, resources and WebMCP tools.
  • Reconstruction obligations, the evidence ledger's unknowns, and capability, provider and session-status listings.
  • Catalog identity (catalogIdentity.ts), client registration and setup environment details, process ownership diagnostics, and the Ghidra transport's exact-key check.

Not included: test files and PeResources.fixture.ts, which are not shipped output. Nothing here was human-facing collation; every site orders machine keys such as IDs, names, paths and environment variable names.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the existing compareUnicodeCodePoints owner; each producer's final ordering step.
  • CLI and MCP/tool-catalog contract: no schema change. The catalog identity digest is unchanged, because the current 172 tool names and 6 prompt names order identically under both comparators (checked with dist/contracts).
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: ordering is now host-independent. Where locale collation and code-point order differ (mixed case, _, digits, non-ASCII), listed items may appear in a different order than before. Set membership is unchanged.
  • Process execution, authorization, cleanup, or containment impact: none.
  • Generated metadata, package, or installation impact: none.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/firmware/firmwareReportFiles.test.ts now inventories a.bin, z.bin, ä.bin, B.bin and Lib/x.bin on the real filesystem through inventoryFirmwareOutput and asserts code-point order.
  • Base reproduction: with only src/firmware/FirmwareFiles.ts reverted to main, the new test fails (1 failed | 5 passed). With the change it passes. The locale table above was produced with node -e under each LC_ALL.
  • User-visible CLI/MCP output: ordering only, as described above.
  • Remaining proof gaps: only firmware has a dedicated regression test. The other sites are the same one-line comparator substitution and are covered by their existing tests.

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

macOS 15 (Darwin 24.3.0, arm64), Node 26.9.0:

  • npm run check:fast: typecheck and lint passed; it also passed as the pre-push hook after rebasing onto 1f531fc2.
  • npm run check:changed: 83 test files, 854 passed, 4 skipped.
  • npm run test:focused -- <all 176 files under tests/boundary/{firmware,filesystem,browser,process}>:
    • 7 Interface Builder / NIB tests fail because ibtool requires full Xcode and this host has only the Command Line Tools. The same 7 fail on unmodified main in a separate worktree.
    • keyedArchiveNativeIntegers (30 s timeout) and processFilesystemCoverageAdapters (process-ownership read) failed once under the full parallel run. Both then passed twice when rerun on this branch.
    • Everything else passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Item sets are unchanged; only locale-dependent ordering becomes code-point ordering.
  • Real Hopper/Ghidra, browser, or OS coverage: no real providers run. macOS only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: comparator substitution only.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • 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/1867 **Author:** [@dollarsmoney](https://github.com/dollarsmoney) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/locale-independent-ordering` --- ### 📝 Commits (2) - [`e16dfc7`](https://github.com/morluto/rea/commit/e16dfc71d9fa0d831ad1e6e1391c69a0c4e69850) fix: order remaining outputs independently of host locale - [`de51642`](https://github.com/morluto/rea/commit/de51642c295acfdeb7bf7c0e71daaa0c9f5fc643) fix(artifacts): order ASAR entries by their portable path ### 📊 Changes **32 files changed** (+174 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `src/application/CapabilityInventory.ts` (+2 -1) 📝 `src/application/ClientConfigurationDocument.ts` (+2 -1) 📝 `src/application/ClientRegistrationEnvironment.ts` (+2 -1) 📝 `src/application/ClientRegistrationStatus.ts` (+2 -1) 📝 `src/application/ReconstructionObligationCandidates.ts` (+3 -2) 📝 `src/application/ReconstructionObligationLedgerDiagnostics.ts` (+3 -1) 📝 `src/application/ReconstructionObligationLedgerEvaluation.ts` (+2 -1) 📝 `src/application/ReconstructionObligationOriginalCases.ts` (+3 -1) 📝 `src/application/SetupPlan.ts` (+2 -1) 📝 `src/application/SetupRegistrationEnvironment.ts` (+2 -1) 📝 `src/application/SupportedClients.ts` (+2 -1) 📝 `src/application/binary/AnalysisProviderRegistry.ts` (+5 -2) 📝 `src/application/binary/BinarySessionStatus.ts` (+11 -4) 📝 `src/application/binary/CompositeProvider.ts` (+5 -4) 📝 `src/application/investigation/EvidenceLedger.ts` (+4 -1) ➕ `src/artifacts/AsarArtifactReader.test.ts` (+25 -0) 📝 `src/artifacts/AsarArtifactReader.ts` (+28 -12) 📝 `src/browser/CdpBrowserProvider.ts` (+4 -1) 📝 `src/browser/CdpCaptureDocuments.ts` (+2 -1) 📝 `src/browser/CdpElectronInspection.ts` (+2 -1) _...and 12 more files_ </details> ### 📄 Description ## Summary `String.prototype.localeCompare()` without a pinned collation follows the host's ICU default locale, which comes from `LANG`/`LC_ALL`. The same input can therefore be ordered differently on different machines. 614fb768 ("fix(evidence): order canonical projections independently of locale") moved the canonical domain projections to `compareUnicodeCodePoints`. Forty-two `localeCompare` calls in 30 provider, application, catalog and setup files still used locale collation. This PR orders all of them by Unicode code point, so no `localeCompare` remains in non-test source. No linked issue: I found this while reading the code, as a direct follow-up to 614fb768. ## Problem and expected behavior Smallest trigger, on unmodified `main`: `inventoryFirmwareOutput` sorts extracted firmware paths with `localeCompare`. ```text LC_ALL=en_US.UTF-8 a.bin ä.bin B.bin z.bin LC_ALL=sv_SE.UTF-8 a.bin B.bin z.bin ä.bin code point B.bin a.bin z.bin ä.bin ``` The violated invariant: evidence and listings derived from the same target should not depend on the analyst's locale. The same kind of drift affects Mach-O symbol lists (`_main` / `_Zeta` / `Zeta` interleave differently from code-point order), and any sorted set that later feeds a digest or comparison. ## Change and scope Every remaining non-test `a.localeCompare(b)` becomes `compareUnicodeCodePoints(a, b)`, the existing owner in `src/domain/unicodeCodePointOrder.ts`. I applied it with a parser-based rewrite and reviewed the result by hand: - Evidence and provider output: Mach-O symbols (`NativeMachoInspection`), firmware extraction paths (`FirmwareFiles`), ASAR entries (`AsarArtifactReader`, previously pinned to `"en"` collation, which still varies by ICU version and is not code-point order), and evidence ID sets (`registerInvestigationTools/helpers`). - Browser and Electron inventories: page capture scripts, capture documents, targets, workers, scripts, resources and WebMCP tools. - Reconstruction obligations, the evidence ledger's unknowns, and capability, provider and session-status listings. - Catalog identity (`catalogIdentity.ts`), client registration and setup environment details, process ownership diagnostics, and the Ghidra transport's exact-key check. Not included: test files and `PeResources.fixture.ts`, which are not shipped output. Nothing here was human-facing collation; every site orders machine keys such as IDs, names, paths and environment variable names. ## Contract and boundary impact - Semantic owner and earliest changed stage: the existing `compareUnicodeCodePoints` owner; each producer's final ordering step. - CLI and MCP/tool-catalog contract: no schema change. The catalog identity digest is unchanged, because the current 172 tool names and 6 prompt names order identically under both comparators (checked with `dist/contracts`). - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: ordering is now host-independent. Where locale collation and code-point order differ (mixed case, `_`, digits, non-ASCII), listed items may appear in a different order than before. Set membership is unchanged. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/firmware/firmwareReportFiles.test.ts` now inventories `a.bin`, `z.bin`, `ä.bin`, `B.bin` and `Lib/x.bin` on the real filesystem through `inventoryFirmwareOutput` and asserts code-point order. - Base reproduction: with only `src/firmware/FirmwareFiles.ts` reverted to `main`, the new test fails (1 failed | 5 passed). With the change it passes. The locale table above was produced with `node -e` under each `LC_ALL`. - User-visible CLI/MCP output: ordering only, as described above. - Remaining proof gaps: only firmware has a dedicated regression test. The other sites are the same one-line comparator substitution and are covered by their existing tests. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed macOS 15 (Darwin 24.3.0, arm64), Node 26.9.0: - `npm run check:fast`: typecheck and lint passed; it also passed as the pre-push hook after rebasing onto `1f531fc2`. - `npm run check:changed`: 83 test files, 854 passed, 4 skipped. - `npm run test:focused -- <all 176 files under tests/boundary/{firmware,filesystem,browser,process}>`: - 7 Interface Builder / NIB tests fail because `ibtool` requires full Xcode and this host has only the Command Line Tools. The same 7 fail on unmodified `main` in a separate worktree. - `keyedArchiveNativeIntegers` (30 s timeout) and `processFilesystemCoverageAdapters` (process-ownership read) failed once under the full parallel run. Both then passed twice when rerun on this branch. - Everything else passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Item sets are unchanged; only locale-dependent ordering becomes code-point ordering. - Real Hopper/Ghidra, browser, or OS coverage: no real providers run. macOS only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: comparator substitution only. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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 00:56:09 +00:00
Sign in to join this conversation.
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#4863
No description provided.