morluto/rea

Fork 0

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

[GH-ISSUE #5872] [Bug] Electron file targets reject encoded separators in query or fragment #3559

New issue

Closed

opened 2026-10-31 00:41:52 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:41:52 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5872

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Process capture or runtime observation

Observed behavior

The built CLI excludes existing local file pages when only their query or fragment contains %2F or %5C. With seven discovery targets pointing to the same regular HTML file, list-electron-targets retains the plain URL and unencoded query/fragment controls, but drops all four encoded-suffix cases and reports excluded.unsupported_url: 4.

Direct inspect-electron-page calls for those four IDs each exit 1 with code: "target_unavailable" and details.reason: "target_not_allowed".

This was executed against a simulated loopback CDP discovery endpoint. No real Electron application was launched. The failure occurs before a WebSocket connection or page capture is attempted.

Reproduction

From the provided revision's built checkout, save the following as reproduce.mjs and run node reproduce.mjs. It creates one inert regular HTML file, supplies seven discovery targets, checks that Node resolves all seven URLs to the same filesystem pathname, and invokes the public CLI. It closes the endpoint and deletes its fixture afterward.

import assert from 'node:assert/strict';
import {execFile} from 'node:child_process';
import {createServer} from 'node:http';
import {mkdtemp, writeFile, rm} from 'node:fs/promises';
import {tmpdir} from 'node:os';
import {join} from 'node:path';
import {pathToFileURL, fileURLToPath} from 'node:url';
import {promisify} from 'node:util';
const run = promisify(execFile);
const root = await mkdtemp(join(process.argv[2] ?? tmpdir(), 'electron-url-'));
const file = join(root, 'index.html');
await writeFile(file, '<!doctype html><title>fixture</title>');
const base = pathToFileURL(file).href;
const suffixes = ['', '?next=/settings', '#/settings', '?next=%2Fsettings', '?next=%5Csettings', '#%2Fsettings', '#%5Csettings'];
let endpoint;
const server = createServer((req, res) => {
  res.setHeader('Content-Type', 'application/json');
  res.end(JSON.stringify(req.url === '/json/version' ? {
    Browser: 'Chrome/fixture', 'Protocol-Version': '1.3',
    'User-Agent': 'fixture', 'V8-Version': 'fixture', 'WebKit-Version': 'fixture',
    webSocketDebuggerUrl: endpoint.replace('http:', 'ws:') + '/devtools/browser/fixture'
  } : suffixes.map((suffix, i) => ({id: `page-${i}`, type: 'page', title: `fixture-${i}`, url: base + suffix, attached: false}))));
});
await new Promise(resolve => server.listen(0, '127.0.0.1', resolve));
endpoint = `http://127.0.0.1:${server.address().port}`;
const cli = async (...args) => {
  try { const r = await run(process.execPath, ['scripts/rea.mjs', ...args, '--json'], {timeout: 20000}); return {code: 0, stdout: JSON.parse(r.stdout), stderr: r.stderr}; }
  catch (e) { return {code: e.code, stdout: JSON.parse(e.stdout), stderr: e.stderr}; }
};
try {
  assert(suffixes.every(s => fileURLToPath(base + s) === file));
  const listed = await cli('list-electron-targets', endpoint);
  const list = listed.stdout.normalized_result;
  console.log('LIST', JSON.stringify({code: listed.code, targets: list.targets.map(t => t.target_id), excluded: list.excluded, stderr: listed.stderr}));
  for (let i = 3; i < suffixes.length; i++) {
    const result = await cli('inspect-electron-page', endpoint, `page-${i}`);
    console.log('INSPECT', suffixes[i], JSON.stringify(result));
  }
} finally {
  await new Promise(resolve => server.close(resolve));
  await rm(root, {recursive: true, force: true});
}

Both runs produced the same exclusions and inspection failures. The endpoint supplies discovery responses only; it does not implement page inspection. This is sufficient to demonstrate erroneous admission before attachment, and does not claim a completed real-Electron capture.

Expected behavior

All seven URLs should be eligible for listing with the same canonical file_path. An encoded separator confined to a query or fragment should not cause direct inspection to fail authorization. Existing rejection of encoded separators in the actual pathname should remain enforced.

The Electron target boundary accepts hostless local file URLs resolving to regular files and rejects encoded path separators. Existing file-scope tests explicitly accept query/hash-bearing URLs and long query text. The executed fileURLToPath equality check establishes that the suffixes in this reproduction do not alter the selected file.

Impact and scope

P2: otherwise eligible renderer targets disappear from discovery and direct inspection is blocked for these bounded URL forms. The effect is a false rejection; no scope bypass, file mutation, or data loss was observed. Real-application frequency is unknown, and no workaround preserving the original renderer URL was tested.

The runtime evidence covers ordinary regular files through the CLI on one platform. Broader effects on ASAR pages, captured frames/resources/scripts/workers, V8 Inspector, and MCP are source-derived possibilities from shared authorization, not executed coverage here.

REA version

6.7.0, source revision 52415975942a4b9c53a817915b313ccc404f280e, using the supplied compiled runtime. Fetched main at f801b2fbfd28f4e3713adefddc7bd73db44c284c has no changes to the relevant authorization, provider, CLI, or target-boundary documentation since that revision.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.22.0. Dependencies and build were supplied with the checkout. No native analysis provider or Electron runtime was used.

Target and artifact context

One inert regular HTML file exposed as seven page targets by a local simulated CDP discovery endpoint. The only difference between target URLs is their query or fragment.

Interface

CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

The listing command exits 0 with empty stderr and this reduced result:

{"targets":["page-0","page-1","page-2"],"excluded":{"unsupported_url":4,"non_page":0}}

All four encoded-suffix inspection commands exit 1 with empty stderr and:

{"code":"target_unavailable","category":"unavailable","message":"Analysis could not complete. Run `rea doctor`, then try again.","retryable":false,"remediation":{"action":"Analysis could not complete. Run `rea doctor`, then try again."},"details":{"operation":"inspect_electron_page","reason":"target_not_allowed"}}

The result was observed twice for each suffix: ?next=%2Fsettings, ?next=%5Csettings, #%2Fsettings, and #%5Csettings. The controls are the plain URL, ?next=/settings, and #/settings.

Typed outcome or recovery expected

Retain all seven eligible targets in discovery. Direct inspection of the encoded-suffix targets should pass file authorization and proceed to attachment. A complete inspection still requires a functioning CDP page transport; the reduced endpoint above intentionally provides only the discovery needed to expose the refusal.

Analysis or suspected cause

Source-derived explanation: ElectronFileScope.ts:111–147 applies /%(?:2f|5c)/iu to the complete input before new URL separates its components. Query and fragment matches therefore return undefined before fileURLToPath and regular-file checks.

The listing consumer counts that result as unsupported_url; the inspection consumer turns it into target_not_allowed. No patch or restoration was tested.

Additional context

Related closed report #2827 and merged PR #2833 cover encoded suffix admission in static HTML script reconstruction, through a different parser. They do not change the passive Electron file-authorization path reproduced here. Merged PR #1400 adds ASAR file authorization and ordinary query/hash support; the whole-input encoded-separator guard remains in the tested revision. A last known good version and real-application prevalence have not been established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5872 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Process capture or runtime observation ### Observed behavior The built CLI excludes existing local file pages when only their query or fragment contains `%2F` or `%5C`. With seven discovery targets pointing to the same regular HTML file, `list-electron-targets` retains the plain URL and unencoded query/fragment controls, but drops all four encoded-suffix cases and reports `excluded.unsupported_url: 4`. Direct `inspect-electron-page` calls for those four IDs each exit 1 with `code: "target_unavailable"` and `details.reason: "target_not_allowed"`. This was executed against a simulated loopback CDP discovery endpoint. No real Electron application was launched. The failure occurs before a WebSocket connection or page capture is attempted. ### Reproduction From the provided revision's built checkout, save the following as `reproduce.mjs` and run `node reproduce.mjs`. It creates one inert regular HTML file, supplies seven discovery targets, checks that Node resolves all seven URLs to the same filesystem pathname, and invokes the public CLI. It closes the endpoint and deletes its fixture afterward. ```js import assert from 'node:assert/strict'; import {execFile} from 'node:child_process'; import {createServer} from 'node:http'; import {mkdtemp, writeFile, rm} from 'node:fs/promises'; import {tmpdir} from 'node:os'; import {join} from 'node:path'; import {pathToFileURL, fileURLToPath} from 'node:url'; import {promisify} from 'node:util'; const run = promisify(execFile); const root = await mkdtemp(join(process.argv[2] ?? tmpdir(), 'electron-url-')); const file = join(root, 'index.html'); await writeFile(file, '<!doctype html><title>fixture</title>'); const base = pathToFileURL(file).href; const suffixes = ['', '?next=/settings', '#/settings', '?next=%2Fsettings', '?next=%5Csettings', '#%2Fsettings', '#%5Csettings']; let endpoint; const server = createServer((req, res) => { res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify(req.url === '/json/version' ? { Browser: 'Chrome/fixture', 'Protocol-Version': '1.3', 'User-Agent': 'fixture', 'V8-Version': 'fixture', 'WebKit-Version': 'fixture', webSocketDebuggerUrl: endpoint.replace('http:', 'ws:') + '/devtools/browser/fixture' } : suffixes.map((suffix, i) => ({id: `page-${i}`, type: 'page', title: `fixture-${i}`, url: base + suffix, attached: false})))); }); await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); endpoint = `http://127.0.0.1:${server.address().port}`; const cli = async (...args) => { try { const r = await run(process.execPath, ['scripts/rea.mjs', ...args, '--json'], {timeout: 20000}); return {code: 0, stdout: JSON.parse(r.stdout), stderr: r.stderr}; } catch (e) { return {code: e.code, stdout: JSON.parse(e.stdout), stderr: e.stderr}; } }; try { assert(suffixes.every(s => fileURLToPath(base + s) === file)); const listed = await cli('list-electron-targets', endpoint); const list = listed.stdout.normalized_result; console.log('LIST', JSON.stringify({code: listed.code, targets: list.targets.map(t => t.target_id), excluded: list.excluded, stderr: listed.stderr})); for (let i = 3; i < suffixes.length; i++) { const result = await cli('inspect-electron-page', endpoint, `page-${i}`); console.log('INSPECT', suffixes[i], JSON.stringify(result)); } } finally { await new Promise(resolve => server.close(resolve)); await rm(root, {recursive: true, force: true}); } ``` Both runs produced the same exclusions and inspection failures. The endpoint supplies discovery responses only; it does not implement page inspection. This is sufficient to demonstrate erroneous admission before attachment, and does not claim a completed real-Electron capture. ### Expected behavior All seven URLs should be eligible for listing with the same canonical `file_path`. An encoded separator confined to a query or fragment should not cause direct inspection to fail authorization. Existing rejection of encoded separators in the actual pathname should remain enforced. The [Electron target boundary](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/electron-observation.md#L24-L34) accepts hostless local file URLs resolving to regular files and rejects encoded *path* separators. Existing [file-scope tests](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/tests/boundary/filesystem/electronFileScope.test.ts#L56-L110) explicitly accept query/hash-bearing URLs and long query text. The executed `fileURLToPath` equality check establishes that the suffixes in this reproduction do not alter the selected file. ### Impact and scope P2: otherwise eligible renderer targets disappear from discovery and direct inspection is blocked for these bounded URL forms. The effect is a false rejection; no scope bypass, file mutation, or data loss was observed. Real-application frequency is unknown, and no workaround preserving the original renderer URL was tested. The runtime evidence covers ordinary regular files through the CLI on one platform. Broader effects on ASAR pages, captured frames/resources/scripts/workers, V8 Inspector, and MCP are source-derived possibilities from shared authorization, not executed coverage here. ### REA version 6.7.0, source revision `52415975942a4b9c53a817915b313ccc404f280e`, using the supplied compiled runtime. Fetched main at `f801b2fbfd28f4e3713adefddc7bd73db44c284c` has no changes to the relevant authorization, provider, CLI, or target-boundary documentation since that revision. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.22.0. Dependencies and build were supplied with the checkout. No native analysis provider or Electron runtime was used. ### Target and artifact context One inert regular HTML file exposed as seven page targets by a local simulated CDP discovery endpoint. The only difference between target URLs is their query or fragment. ### Interface CLI ### Earliest failure stage Setup or provider admission ### Relevant output or evidence The listing command exits 0 with empty stderr and this reduced result: ```json {"targets":["page-0","page-1","page-2"],"excluded":{"unsupported_url":4,"non_page":0}} ``` All four encoded-suffix inspection commands exit 1 with empty stderr and: ```json {"code":"target_unavailable","category":"unavailable","message":"Analysis could not complete. Run `rea doctor`, then try again.","retryable":false,"remediation":{"action":"Analysis could not complete. Run `rea doctor`, then try again."},"details":{"operation":"inspect_electron_page","reason":"target_not_allowed"}} ``` The result was observed twice for each suffix: `?next=%2Fsettings`, `?next=%5Csettings`, `#%2Fsettings`, and `#%5Csettings`. The controls are the plain URL, `?next=/settings`, and `#/settings`. ### Typed outcome or recovery expected Retain all seven eligible targets in discovery. Direct inspection of the encoded-suffix targets should pass file authorization and proceed to attachment. A complete inspection still requires a functioning CDP page transport; the reduced endpoint above intentionally provides only the discovery needed to expose the refusal. ### Analysis or suspected cause Source-derived explanation: [ElectronFileScope.ts:111–147](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/ElectronFileScope.ts#L111-L147) applies `/%(?:2f|5c)/iu` to the complete input before `new URL` separates its components. Query and fragment matches therefore return `undefined` before `fileURLToPath` and regular-file checks. The [listing consumer](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/CdpElectronProvider.ts#L146-L155) counts that result as `unsupported_url`; the [inspection consumer](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/CdpElectronProvider.ts#L423-L430) turns it into `target_not_allowed`. No patch or restoration was tested. ### Additional context Related closed report #2827 and merged PR #2833 cover encoded suffix admission in static HTML script reconstruction, through a different parser. They do not change the passive Electron file-authorization path reproduced here. Merged PR #1400 adds ASAR file authorization and ordinary query/hash support; the whole-input encoded-separator guard remains in the tested revision. A last known good version and real-application prevalence have not been established.

bsakaguchi closed this issue

2026-10-31 00:41:52 +00:00

bsakaguchi commented

2026-10-31 00:41:53 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

Encoded-separator rejection now examines URL.pathname rather than the full URL, so query and fragment encodings do not affect file authorization. The regression covers all four encoded suffixes and retains pathname-encoded separator rejection. fileURLToPath still supplies the same canonical local file.

Implementation: ElectronFileScope.ts.

The relevant regression checks in electronFileScope.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. Encoded-separator rejection now examines URL.pathname rather than the full URL, so query and fragment encodings do not affect file authorization. The regression covers all four encoded suffixes and retains pathname-encoded separator rejection. fileURLToPath still supplies the same canonical local file. Implementation: [ElectronFileScope.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/ElectronFileScope.ts#L136). The relevant regression checks in [electronFileScope.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/filesystem/electronFileScope.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

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

[PR #3559] [CLOSED] fix(electron): block and record shell.trashItem #5823

bsakaguchi referenced this issue

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

[PR #4562] [MERGED] fix(electron): block and record synchronous shell effects in the active hook #6308

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425