Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/reviews/2026-10-31-browser-mirror-guards.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Browser mirror CLI guard review

One fresh read-only whole-change review used gpt-6-astra/high on 59e2e19..fa17609. No Critical, Important or Minor findings. The reviewer independently ran all 12 tests under Node24.22.1 with zero failures, cancellations or skips; git diff --check passed. No additional review pass or code correction was required.

The port validates the exact raw URL token subsequently emitted and preserves supported transport, ordering, metadata and Azure fallback. Exclusive new output creation refuses existing files, symbolic links and hard links without modifying input; validation finishes before creation. Mode0600 is compatible with the existing subsequent sudo install. The three call sites belong to separate browser-investigation, portable-replay and e2e Ubuntu jobs, including independent matrix jobs. An earlier narrative incorrectly named the third job integration; e2e is correct.

Root's baseline7 tests passed. Five added actual regressions failed against unmodified main, then the corrected helper passed all12. The existing configured Prettier3.9.10 binary was used with this repository's configuration; no dependency installation, consumer qualification, local /etc operation or release occurred.

The review declined hosted mirror availability/performance/TLS/package-install outcomes; unchanged application/browser functionality; alternative filesystems or hostile concurrent replacement outside trusted runner temporary directories; and formatter-install/dependency provenance qualification. Root preserves these as explicit evidence limits. The contract is for the observed local Ubuntu filesystem and trusted runner inputs, not customer isolation or arbitrary filesystem security. Required latest-head hosted CI remains mandatory before merge; no speed or live package-security guarantee is claimed. No deferred minor findings.
38 changes: 38 additions & 0 deletions docs/superpowers/plans/2026-10-31-browser-mirror-guards.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Browser mirror CLI guards Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans for this native maintenance task.

**Goal:** Port the confirmed mirror URL/output safety corrections to the original OSS helper.

**Architecture:** Same Node-only helper and existing test/CI entrypoints. No package, framework, dependency, workflow permission, release or local system change.

**Tech Stack:** Existing Node builtins and configured Prettier3.9.10.

**Spec:** This small plan contains the complete maintenance contract; continuous-build authority selects native execution.

## Global Constraints

- Fresh remote main59e2e19; isolated .worktrees/domscribe-ci-mirror-guards-20261031, preserve all primary/user work.
- Preserve current archive/security/Azure order, fallback, transport, metadata, package signatures and generic validation errors.
- Validate exact raw HTTP/HTTPS official host and /ubuntu[/] syntax, refusing normalization-dependent ports, paths, queries/fragments, userinfo and encoded hosts.
- Output must be a new file, created exclusively; symlink/hardlink/existing-output aliases cannot overwrite input. Manual retry chooses another output path.
- Existing three workflow preparations belong to three distinct jobs; each has fresh RUNNER_TEMP, so the new exclusive-output contract fits all callers.
- Evidence uses only owned temporary files. No local /etc/sudo, cloud, credentials, dependency changes or publication. OSS standing normalmerge/latestCI/review, admin only if review rules otherwise block our PR.

## Review Focus

Same-byte URL validation/emission; input/output alias safety; generic errors preserve privacy; all workflow caller output paths are fresh; Node/runtime formatting compatibility without application changes.

### Task 1: Safe raw mirror parsing and exclusive output

**Files:** scripts/fixtures/browser-mirrors.mjs and browser-mirrors.test.mjs.

**Interfaces:** Existing preferUbuntuMirrors(content) and input/output CLI. Reuse reviewed/fixed managed helper policy without a new public interface.

- [ ] Baseline7 tests pass; copied five new regressions fail on actual current OSS helper (12total/7PASS/5FAIL). Retain real output.
- [ ] Port exact fixed helper; run12 tests GREEN and formatter3.9.10 with this repo config. Existing installed formatter may be used explicitly; no new install/consumer qualification claimed. No full unchanged application suite locally; latest hosted required CI remains mandatory.
- [ ] One fresh most-capable read-only whole-change review; one Root regression-backed material pass if required, no re-review loop. Push ready focused PR/attach; latest-head required checks pass; normal merge preferably, standing admin only if otherwise blocked, exacttree/postmain/nativegate. Do not publish or deploy.

## Self-review

The managed adaptation independently confirmed both latent defects. This port repairs the original helper instead of leaving it behind; all original positive/negative tests and caller behavior remain, with deliberate refusal of existing output. Source-only local tests do not establish hosted speed, TLS, mirror availability or application functionality.
32 changes: 12 additions & 20 deletions scripts/fixtures/browser-mirrors.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,18 @@ export function preferUbuntuMirrors(content) {
}
const match = line.match(/^(https?:\/\/[^\s]+)\t(.+)$/);
if (!match) unsupported();
let uri;
try {
uri = new URL(match[1]);
} catch {
// Native URL errors retain input; do not echo unfamiliar runner data.
unsupported();
}
// Validate the bytes we emit, not a URL parser's normalized spelling.
const host = match[1].match(
/^https?:\/\/(archive\.ubuntu\.com|security\.ubuntu\.com|azure\.archive\.ubuntu\.com)\/ubuntu\/?$/,
)?.[1];
if (!host) unsupported();
const metadata = match[2].trim().split(/\s+/);
const priorities = metadata.filter((value) =>
value.startsWith('priority:'),
);
if (
!order.includes(uri.hostname) ||
uri.username ||
uri.password ||
uri.search ||
uri.hash ||
uri.port ||
!/^\/ubuntu\/?$/.test(uri.pathname) ||
priorities.length !== 1 ||
!/^priority:\d+$/.test(priorities[0])
)
if (priorities.length !== 1 || !/^priority:\d+$/.test(priorities[0]))
unsupported();
rows.push({ uri: match[1], host: uri.hostname, metadata });
rows.push({ uri: match[1], host, metadata });
}
if (
rows.length !== order.length ||
Expand Down Expand Up @@ -72,5 +60,9 @@ if (
'Usage: browser-mirrors.mjs <runner-mirror-list> <prepared-list>',
);
}
writeFileSync(output, preferUbuntuMirrors(readFileSync(input, 'utf8')));
// Exclusive creation refuses existing files and filesystem aliases atomically.
writeFileSync(output, preferUbuntuMirrors(readFileSync(input, 'utf8')), {
flag: 'wx',
mode: 0o600,
});
}
104 changes: 103 additions & 1 deletion scripts/fixtures/browser-mirrors.test.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import assert from 'node:assert/strict';
import { spawnSync } from 'node:child_process';
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import {
mkdtempSync,
readFileSync,
rmSync,
writeFileSync,
symlinkSync,
linkSync,
} from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import test from 'node:test';
Expand Down Expand Up @@ -125,3 +132,98 @@ test('invalid CLI input fails without replacing an existing prepared copy', () =
rmSync(root, { recursive: true });
}
});

test('reject raw URL spellings that a URL parser would normalize into the allowlist', () => {
for (const url of [
'http://archive.ubuntu.com:80/ubuntu',
'https://archive.ubuntu.com:443/ubuntu',
'http://archive.ubuntu.com/unexpected/../ubuntu',
'http://archive.ubuntu.com/ubuntu?',
'http://archive.ubuntu.com/ubuntu#',
'http://@archive.ubuntu.com/ubuntu',
'http://%61rchive.ubuntu.com/ubuntu',
])
assert.throws(
() =>
preferUbuntuMirrors(
runnerList.replace('http://archive.ubuntu.com/ubuntu', url),
),
{ message: 'Unsupported Ubuntu runner mirror list' },
);
});
test('CLI rejects normalized URL tokens without replacing existing output', () => {
const root = mkdtempSync(join(tmpdir(), 'ds-mirror-normalized-'));
try {
const input = join(root, 'input'),
output = join(root, 'output');
writeFileSync(
input,
runnerList.replace(
'http://archive.ubuntu.com/ubuntu',
'http://archive.ubuntu.com:80/ubuntu',
),
);
writeFileSync(output, 'unchanged');
const result = spawnSync(
process.execPath,
[
new URL('./browser-mirrors.mjs', import.meta.url).pathname,
input,
output,
],
{ encoding: 'utf8' },
);
assert.notEqual(result.status, 0);
assert.equal(readFileSync(output, 'utf8'), 'unchanged');
} finally {
rmSync(root, { recursive: true, force: true });
}
});
for (const kind of ['symlink', 'hardlink'])
test('CLI refuses ' + kind + ' output alias without modifying input', () => {
const root = mkdtempSync(join(tmpdir(), 'ds-mirror-alias-'));
try {
const input = join(root, 'input'),
output = join(root, 'output');
writeFileSync(input, runnerList);
if (kind === 'symlink') symlinkSync(input, output);
else linkSync(input, output);
const result = spawnSync(
process.execPath,
[
new URL('./browser-mirrors.mjs', import.meta.url).pathname,
input,
output,
],
{ encoding: 'utf8' },
);
assert.notEqual(result.status, 0);
assert.equal(readFileSync(input, 'utf8'), runnerList);
assert.equal(readFileSync(output, 'utf8'), runnerList);
} finally {
rmSync(root, { recursive: true, force: true });
}
});
test('CLI requires a new output even for valid input', () => {
const root = mkdtempSync(join(tmpdir(), 'ds-mirror-existing-'));
try {
const input = join(root, 'input'),
output = join(root, 'output');
writeFileSync(input, runnerList);
writeFileSync(output, 'keep');
const result = spawnSync(
process.execPath,
[
new URL('./browser-mirrors.mjs', import.meta.url).pathname,
input,
output,
],
{ encoding: 'utf8' },
);
assert.notEqual(result.status, 0);
assert.equal(readFileSync(output, 'utf8'), 'keep');
assert.equal(readFileSync(input, 'utf8'), runnerList);
} finally {
rmSync(root, { recursive: true, force: true });
}
});
Loading