morluto/rea

Fork 0

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

[PR #1787] [CLOSED] fix(javascript): percent-decode local URL references once #4818

New issue

Closed

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

bsakaguchi commented

2026-10-31 00:55:21 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1787
Author: @falkoro
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/1782-decode-local-url-references

📝 Commits (1)

8912e45 fix(javascript): percent-decode local URL references once

📊 Changes

3 files changed (+83 additions, -0 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+40 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+8 -0)
📝 tests/boundary/filesystem/javascriptSourceMapLocations.test.ts (+35 -0)

📄 Description

Summary

Fixes #1782. Local source-map URLs were looked up without percent-decoding. //# sourceMappingURL=app%20part.js.map never matched the inventoried app part.js.map, so the reference was reported as resolved_path: null, available: false.

Problem and expected behavior

For url-reference, contextualCandidate strips the query and fragment but passed the still-encoded path to relativeCandidate. The HTML and ESM branches already decode. A relative URL path names the file its decoded bytes spell, decoded exactly once: %2520 is a file named %20, not a space. The declared URL and its digest must stay as written.

Change and scope

After the external-URL check, url-reference paths are decoded once with percentDecodeUrlPath, the URL path decoder already used for HTML references. A path that decodes to NUL or invalid UTF-8 is rejected, as in the HTML branch. Encoded dot, separator and backslash bytes are still rejected earlier by rejectDeclaration, and confinement runs on the decoded candidate. declared_url, its digest and location are unchanged.

url-reference also resolves new Worker(...) and navigator.serviceWorker.register(...) script URLs, which get the same decoding. They are URLs with the same semantics, so the fix stays in the shared resolver.

Contract and boundary impact

Semantic owner and earliest changed stage: url-reference resolution in src/application/javascript/JavaScriptArtifactPathResolution.ts.
CLI and MCP/tool-catalog contract: none; existing resolved_path and available values become correct.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: raw declared_url and declared_url_sha256 are preserved; only the lookup path is decoded.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

Tests added or updated:
- src/application/javascript/JavaScriptArtifactPathResolution.test.ts: %20, %25, %2520 (single decode, with both the space and the literal %20 file inventoried) and %20 with ?v=1#section. %00 and %FF are rejected; %2e%2e/ stays rejected.
- tests/boundary/filesystem/javascriptSourceMapLocations.test.ts: reconstruction with app%20part.js.map?v=1#section and a real app part.js.map. Asserts the raw declared URL and digest, resolved_path: "app part.js.map" and available: true.
Base reproduction: on 85ccef34 7 of the 8 new cases fail; the %2e%2e/ control already passed.
User-visible CLI output (built CLI, issue fixtures, --max-heap-mb 512 --json), source-map reference observation:

Declared URL	File on disk	Before	After
`app%20part.js.map`	`app part.js.map`	`null`, `false`	`app part.js.map`, `true`
`app%25part.js.map`	`app%part.js.map`	`null`, `false`	`app%part.js.map`, `true`
`app%2520part.js.map`	`app%20part.js.map`	`null`, `false`	`app%20part.js.map`, `true`
`app%20part.js.map?v=1#section`	`app part.js.map`	`null`, `false`	`app part.js.map`, `true`

declared_url is the raw text in every row, before and after.

Remaining proof gaps: encoded worker and service-worker URLs are covered through the shared resolver tests only, not a separate workflow test. Windows and macOS were not run.

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

Linux x64, Node 24.18.0, npm 11.16.0:

npm run test:focused -- all src/application/javascript/*.test.ts, src/domain/javascript/*.test.ts, tests/boundary/javascript/*.test.ts, tests/boundary/filesystem/javascript*.test.ts, tests/boundary/filesystem/sourceToBundlePaths.test.ts and tests/composition/javascript-applications/*.test.ts: 91 files, 1531 tests passed.
npm run check: passed (0 lint errors, warning count unchanged from main; architecture guards passed).
npm run verify:test-discovery: passed.
npm run build:cached, then node scripts/rea.mjs analyze-javascript-application <fixture> --max-heap-mb 512 --json for the four fixtures in the table.
git diff --check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. One edge changes: a file literally named app%20part.js.map is no longer matched by the declaration app%20part.js.map. It is matched by app%2520part.js.map, which is what a browser or static server would load.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; Linux only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: decoding happens after the existing encoded-dot/separator rejection and before root confinement, so a decoded path cannot escape the artifact root.

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.

🤖 Generated with Claude Code

_{🔄 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/1787 **Author:** [@falkoro](https://github.com/falkoro) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/1782-decode-local-url-references` --- ### 📝 Commits (1) - [`8912e45`](https://github.com/morluto/rea/commit/8912e451ca18e4f006ff60e551dfed95e699e2fd) fix(javascript): percent-decode local URL references once ### 📊 Changes **3 files changed** (+83 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+40 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+8 -0) 📝 `tests/boundary/filesystem/javascriptSourceMapLocations.test.ts` (+35 -0) </details> ### 📄 Description ## Summary Fixes #1782. Local source-map URLs were looked up without percent-decoding. `//# sourceMappingURL=app%20part.js.map` never matched the inventoried `app part.js.map`, so the reference was reported as `resolved_path: null`, `available: false`. ## Problem and expected behavior For `url-reference`, `contextualCandidate` strips the query and fragment but passed the still-encoded path to `relativeCandidate`. The HTML and ESM branches already decode. A relative URL path names the file its decoded bytes spell, decoded exactly once: `%2520` is a file named `%20`, not a space. The declared URL and its digest must stay as written. ## Change and scope After the external-URL check, `url-reference` paths are decoded once with `percentDecodeUrlPath`, the URL path decoder already used for HTML references. A path that decodes to NUL or invalid UTF-8 is `rejected`, as in the HTML branch. Encoded dot, separator and backslash bytes are still rejected earlier by `rejectDeclaration`, and confinement runs on the decoded candidate. `declared_url`, its digest and location are unchanged. `url-reference` also resolves `new Worker(...)` and `navigator.serviceWorker.register(...)` script URLs, which get the same decoding. They are URLs with the same semantics, so the fix stays in the shared resolver. ## Contract and boundary impact - Semantic owner and earliest changed stage: `url-reference` resolution in `src/application/javascript/JavaScriptArtifactPathResolution.ts`. - CLI and MCP/tool-catalog contract: none; existing `resolved_path` and `available` values become correct. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: raw `declared_url` and `declared_url_sha256` are preserved; only the lookup path is decoded. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: - `src/application/javascript/JavaScriptArtifactPathResolution.test.ts`: `%20`, `%25`, `%2520` (single decode, with both the space and the literal `%20` file inventoried) and `%20` with `?v=1#section`. `%00` and `%FF` are `rejected`; `%2e%2e/` stays rejected. - `tests/boundary/filesystem/javascriptSourceMapLocations.test.ts`: reconstruction with `app%20part.js.map?v=1#section` and a real `app part.js.map`. Asserts the raw declared URL and digest, `resolved_path: "app part.js.map"` and `available: true`. - Base reproduction: on `85ccef34` 7 of the 8 new cases fail; the `%2e%2e/` control already passed. - User-visible CLI output (built CLI, issue fixtures, `--max-heap-mb 512 --json`), source-map reference observation: | Declared URL | File on disk | Before | After | | --- | --- | --- | --- | | `app%20part.js.map` | `app part.js.map` | `null`, `false` | `app part.js.map`, `true` | | `app%25part.js.map` | `app%part.js.map` | `null`, `false` | `app%part.js.map`, `true` | | `app%2520part.js.map` | `app%20part.js.map` | `null`, `false` | `app%20part.js.map`, `true` | | `app%20part.js.map?v=1#section` | `app part.js.map` | `null`, `false` | `app part.js.map`, `true` | `declared_url` is the raw text in every row, before and after. - Remaining proof gaps: encoded worker and service-worker URLs are covered through the shared resolver tests only, not a separate workflow test. Windows and macOS were not run. 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 Linux x64, Node 24.18.0, npm 11.16.0: - `npm run test:focused --` all `src/application/javascript/*.test.ts`, `src/domain/javascript/*.test.ts`, `tests/boundary/javascript/*.test.ts`, `tests/boundary/filesystem/javascript*.test.ts`, `tests/boundary/filesystem/sourceToBundlePaths.test.ts` and `tests/composition/javascript-applications/*.test.ts`: 91 files, 1531 tests passed. - `npm run check`: passed (0 lint errors, warning count unchanged from main; architecture guards passed). - `npm run verify:test-discovery`: passed. - `npm run build:cached`, then `node scripts/rea.mjs analyze-javascript-application <fixture> --max-heap-mb 512 --json` for the four fixtures in the table. - `git diff --check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. One edge changes: a file literally named `app%20part.js.map` is no longer matched by the declaration `app%20part.js.map`. It is matched by `app%2520part.js.map`, which is what a browser or static server would load. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; Linux only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: decoding happens after the existing encoded-dot/separator rejection and before root confinement, so a decoded path cannot escape the artifact root. ## 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:21 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns