From 70dca86bc11c483289ada854d0bbd16b87d87885 Mon Sep 17 00:00:00 2001 From: Bilawal Sidhu <106619546+bilawalsidhu@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:34:23 -0500 Subject: [PATCH] fix(pinokio): disclose incoming changes before Update pulls and installs (#507) Supersedes #356 by @Lob26, ported onto the current component layout with fixes from review. Verified before merge: format, package boundaries, unit suite, build, and the browser tracking suite; a browser harness for the change where one applies; adversarial review applied. Co-authored-by: Pedro Lobato <69770518+Lob26@users.noreply.github.com> --- CHANGELOG.md | 10 + scripts/format-scope.json | 4 +- scripts/pinokio-update.mjs | 240 +++++++++++++++- src/pinokioUpdatePreview.test.mjs | 447 ++++++++++++++++++++++++++++++ 4 files changed, 697 insertions(+), 4 deletions(-) create mode 100644 src/pinokioUpdatePreview.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index a5a229e13..790aa2740 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -253,6 +253,16 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md last good picture kept after a failed refresh — and the one feature that sends imagery anywhere: the voice assistant's viewport screenshot. Contributed by Lob26 (#357). +- Pinokio's Update shows what it is about to install before it installs it: the + tracking branch, the remote it fetched from, the incoming commits and their + diffstat. The remote is printed as host and path — a password or token in the + URL's user field is replaced and any query string dropped, though a secret + spelled as an ordinary path segment cannot be told from a repository name. It + fetches once and applies exactly the revision it named, so a commit that lands + mid-update cannot be installed unannounced. A git read it cannot complete is + reported as such instead of as "already up to date", and if the revision to + apply cannot be resolved at all the update stops without installing anything + and exits unsuccessfully. Contributed by Lob26 (#356). - Extract panel disclosure and hover/focus controls into a reusable module; cancel their listeners and pending work during replacement and teardown. diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 7c192c15e..10a18fd63 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -615,5 +615,7 @@ "src/cityPresets.test.mjs", "server/providers/cctv/range.js", "src/data/cctvMediaRange.test.mjs", - "src/materialSymbolsSubset.test.mjs" + "src/materialSymbolsSubset.test.mjs", + "scripts/pinokio-update.mjs", + "src/pinokioUpdatePreview.test.mjs" ] diff --git a/scripts/pinokio-update.mjs b/scripts/pinokio-update.mjs index 22d19b886..1259179c7 100644 --- a/scripts/pinokio-update.mjs +++ b/scripts/pinokio-update.mjs @@ -1,5 +1,239 @@ #!/usr/bin/env node -import { installPinokioDependencies, runChecked } from './pinokio-install.mjs'; +/** + * Pinokio "Update": disclose what is about to land, then fast-forward and reinstall. + * + * Update is a fixed menu item aimed at people who only want to run the app, and + * it executes install scripts from whatever the configured remote serves. It + * cannot ask for confirmation — Pinokio drives it non-interactively — so the + * safety it can offer is disclosure: fetch once, print the remote it fetched + * from plus the incoming commits and their diffstat, and then apply that exact + * revision. Someone who sees an unfamiliar remote or an unexpected set of + * commits can close the window before any install script runs. + * + * @module scripts/pinokio-update + */ +import { realpathSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + installPinokioDependencies, + isDirectInvocation, + runChecked, +} from './pinokio-install.mjs'; -runChecked('git', ['pull', '--ff-only']); -installPinokioDependencies(); +const MODULE_PATH = fileURLToPath(import.meta.url); +const ROOT = realpathSync(path.resolve(path.dirname(MODULE_PATH), '..')); + +/** + * Read a git value. + * + * @param {string[]} args - git arguments. + * @param {object} [options] + * @param {string} [options.cwd] - Repository to read. + * @returns {string|null} Trimmed stdout, `''` when git succeeded with no + * output, or null when git failed (no upstream, detached HEAD, not a + * checkout). Callers must keep those two apart: an empty range and an + * unreadable one mean very different things to someone about to install. + */ +export function readGit(args, { cwd = ROOT } = {}) { + const result = spawnSync('git', args, { cwd, encoding: 'utf8' }); + if (result.error || result.status !== 0) return null; + return result.stdout.trim(); +} + +/** + * Remove credentials from a remote URL before it is printed. + * + * A remote can carry a password or an access token in its userinfo + * (`https://token@host/org/repo.git`) or in a query parameter + * (`?access_token=…`). The point of printing the remote is to show WHERE the + * update comes from, which the host and path already say — so the userinfo is + * replaced and the query string and fragment are dropped whole, rather than + * matched against a list of parameter names that would miss the next one. + * + * A secret spelled as an ordinary path segment is not detected; nothing here + * can tell one of those from a repository name. + * + * @param {string|null|undefined} url - Remote URL as git reports it. + * @returns {string|null} Printable URL, or null when there is nothing to print. + */ +export function redactRemoteUrl(url) { + if (typeof url !== 'string' || url.length === 0) return null; + let parsed; + try { + parsed = new URL(url); + } catch { + // Not a URL: scp-style `git@host:org/repo.git`, or a local path. Neither + // has a userinfo field to hide. + return url; + } + const hadUserinfo = Boolean(parsed.username || parsed.password); + const hadQuery = Boolean(parsed.search || parsed.hash); + if (!hadUserinfo && !hadQuery) return url; + parsed.password = ''; + if (hadUserinfo) parsed.username = '***'; + parsed.search = ''; + parsed.hash = ''; + return parsed.toString(); +} + +/** + * Print the remote, the commits about to be applied, and their diffstat. + * + * @param {object} [io] - Injection seam for tests. + * @param {string} [io.cwd] - Repository to inspect. + * @param {(line: string) => void} [io.log] - Normal output. + * @param {(line: string) => void} [io.warn] - Degraded-path output. + * @param {(remote: string) => void} [io.fetchRemote] - Fetch step; throws or + * exits on failure, because a report built on a stale remote ref describes + * the wrong changes. + * @param {(args: string[]) => (string|null)} [io.read] - git reader. + * @returns {{apply: string|null, fallback: boolean, stopped?: boolean}} + * `apply` is the exact revision that was disclosed and must now be applied. + * `fallback` asks the caller to run the plain pull so git itself reports + * whatever went wrong, and is only ever set before anything has been fetched. + * `stopped` means the update gave up: nothing can be shown, so nothing is + * applied. + */ +export function reportIncomingChanges(io = {}) { + const { + cwd = ROOT, + log = (line) => console.log(line), + warn = (line) => console.warn(line), + fetchRemote = (remote) => runChecked('git', ['fetch', '--quiet', remote]), + read = (args) => readGit(args, { cwd }), + } = io; + + const upstream = read([ + 'rev-parse', + '--abbrev-ref', + '--symbolic-full-name', + '@{u}', + ]); + // The full ref name is what later reads resolve, so a remote whose own name + // contains a slash (`team/origin`) cannot be mistaken for a path segment. + const upstreamRef = read(['rev-parse', '--symbolic-full-name', '@{u}']); + const branch = read(['rev-parse', '--abbrev-ref', 'HEAD']); + const remote = + branch && branch !== 'HEAD' + ? read(['config', '--get', `branch.${branch}.remote`]) + : null; + if (!upstream || !upstreamRef || !remote) { + warn('[update] No upstream branch is configured — cannot preview changes.'); + warn('[update] Continuing; `git pull --ff-only` will report the problem.'); + return { apply: null, fallback: true }; + } + + log(`[update] Tracking ${upstream}`); + if (remote === '.') { + // The upstream is another branch in this same checkout; there is nothing + // to fetch and no remote URL to disclose. + log('[update] Upstream is a local branch — nothing to fetch.'); + } else { + const url = redactRemoteUrl(read(['remote', 'get-url', remote])); + log(`[update] Fetching from ${url || remote}`); + // Fetch once, here. Everything below describes and then applies the + // revision this fetch brought in; a second fetch inside `git pull` could + // apply something newer than what was printed. + fetchRemote(remote); + } + + const target = read(['rev-parse', upstreamRef]); + const head = read(['rev-parse', 'HEAD']); + if (!target || !head) { + // Stop. Falling through to `git pull` here would fetch a second time and + // could install a revision this run never resolved, let alone disclosed. + warn(`[update] Could not resolve ${upstream} after fetching.`); + warn( + '[update] Stopping: nothing is applied, because nothing can be shown.', + ); + return { apply: null, fallback: false, stopped: true }; + } + if (target === head) { + log('[update] Already up to date — reinstalling dependencies only.'); + return { apply: null, fallback: false }; + } + + const commits = read([ + 'log', + '--oneline', + '--no-decorate', + `${head}..${target}`, + ]); + if (commits === null) { + // A read that failed is not an empty range. Saying "up to date" here would + // tell someone nothing is arriving while a revision is about to be applied. + warn(`[update] Could not list the commits in ${head}..${target}.`); + warn( + `[update] Applying ${target} anyway; it is the revision that was fetched.`, + ); + return { apply: target, fallback: false }; + } + + const lines = commits.length > 0 ? commits.split('\n') : []; + if (lines.length > 0) { + log(`\n[update] ${lines.length} incoming commit(s):`); + for (const line of lines) log(` ${line}`); + } else { + log( + `\n[update] ${upstream} is at ${target}, which is not ahead of this checkout.`, + ); + } + + const stat = read(['diff', '--stat', `${head}..${target}`]); + if (stat === null) { + warn(`[update] Could not read the diffstat for ${head}..${target}.`); + } else if (stat) { + log('\n[update] Files affected:'); + for (const line of stat.split('\n')) log(` ${line}`); + } + log(`\n[update] Applying ${target}, then reinstalling dependencies.\n`); + return { apply: target, fallback: false }; +} + +/** + * Disclose, then apply exactly what was disclosed. + * + * @param {object} [io] - Same seam as {@link reportIncomingChanges}, plus + * `apply`, which runs one git command and must fail loudly. + * @returns {{apply: string|null, fallback: boolean}} What the report decided. + */ +export function updateFromRemote(io = {}) { + const { apply = (args) => runChecked('git', args) } = io; + const plan = reportIncomingChanges(io); + // `merge --ff-only ` applies the object the report named. A second + // `pull` would fetch again and could land a different one. + if (plan.apply) apply(['merge', '--ff-only', plan.apply]); + else if (plan.fallback) apply(['pull', '--ff-only']); + return plan; +} + +/** + * What running this script does: disclose, apply, install. + * + * @param {object} [io] - The seam above, plus `install` and `fail`. + * @returns {{apply: string|null, fallback: boolean, stopped?: boolean}} The plan. + */ +export function runPinokioUpdate(io = {}) { + const { + install = installPinokioDependencies, + fail = (code) => process.exit(code), + } = io; + const plan = updateFromRemote(io); + if (plan.stopped) { + // Nothing could be shown, so nothing was applied. Reinstalling now would + // run install scripts under the banner of an update that did not happen, + // and report success for it. + fail(1); + return plan; + } + install(); + return plan; +} + +// Guarded like pinokio-start.mjs so the report above can be exercised without +// pulling and reinstalling as an import side effect. +if (isDirectInvocation(process.argv[1], MODULE_PATH)) { + runPinokioUpdate(); +} diff --git a/src/pinokioUpdatePreview.test.mjs b/src/pinokioUpdatePreview.test.mjs new file mode 100644 index 000000000..ca3cc7688 --- /dev/null +++ b/src/pinokioUpdatePreview.test.mjs @@ -0,0 +1,447 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync, writeFileSync } from 'node:fs'; +import { mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; +import { spawnSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { + readGit, + redactRemoteUrl, + reportIncomingChanges, + runPinokioUpdate, + updateFromRemote, +} from '../scripts/pinokio-update.mjs'; + +const source = readFileSync( + new URL('../scripts/pinokio-update.mjs', import.meta.url), + 'utf8', +); + +/** Run git in a fixture repository, failing the test on a git error. */ +function git(cwd, args) { + const result = spawnSync('git', args, { cwd, encoding: 'utf8' }); + assert.equal( + result.status, + 0, + `git ${args.join(' ')} failed: ${result.stderr || result.error}`, + ); + return result.stdout.trim(); +} + +/** Escape a path so it can be matched literally inside a RegExp. */ +const literal = (value) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +/** + * Build a disposable origin repository and a clone that tracks it. + * + * Everything is on disk and driven through real git, so the assertions are + * about what the update path does to a checkout rather than about a transcript + * of injected strings. + * + * @param {import('node:test').TestContext} t - Owns the fixture's cleanup. + */ +async function fixture(t) { + const scratch = await realpath( + await mkdtemp(path.join(tmpdir(), 'gev-update-')), + ); + t.after(() => rm(scratch, { recursive: true, force: true })); + const origin = path.join(scratch, 'origin'); + const clone = path.join(scratch, 'clone'); + + git(scratch, ['init', '--quiet', '--initial-branch=main', origin]); + git(origin, ['config', 'user.email', 'fixture@example.invalid']); + git(origin, ['config', 'user.name', 'Fixture']); + await writeFile(path.join(origin, 'README.md'), 'first\n'); + git(origin, ['add', 'README.md']); + git(origin, ['commit', '--quiet', '-m', 'first commit']); + + git(scratch, ['clone', '--quiet', origin, clone]); + git(clone, ['config', 'user.email', 'fixture@example.invalid']); + git(clone, ['config', 'user.name', 'Fixture']); + + /** Add one commit to the origin repository. */ + const push = async (message, contents) => { + await writeFile(path.join(origin, 'README.md'), contents); + git(origin, ['add', 'README.md']); + git(origin, ['commit', '--quiet', '-m', message]); + return git(origin, ['rev-parse', 'HEAD']); + }; + + /** Same, from a synchronous callback (used to move the remote mid-update). */ + const pushSync = (message, contents) => { + writeFileSync(path.join(origin, 'README.md'), contents); + git(origin, ['add', 'README.md']); + git(origin, ['commit', '--quiet', '-m', message]); + return git(origin, ['rev-parse', 'HEAD']); + }; + + const out = []; + const io = { + cwd: clone, + log: (line) => out.push(String(line)), + warn: (line) => out.push(String(line)), + fetchRemote: (remote) => git(clone, ['fetch', '--quiet', remote]), + apply: (args) => git(clone, args), + }; + + return { + scratch, + origin, + clone, + push, + pushSync, + io, + out, + text: () => out.join('\n'), + }; +} + +test('a checkout with no upstream is reported and left to the plain pull', async (t) => { + const app = await fixture(t); + git(app.clone, ['checkout', '--quiet', '-b', 'local-only']); + let fetched = null; + const plan = reportIncomingChanges({ + ...app.io, + fetchRemote: (remote) => { + fetched = remote; + }, + }); + assert.deepEqual(plan, { apply: null, fallback: true }); + assert.match(app.text(), /No upstream branch is configured/); + assert.equal(fetched, null, 'nothing is fetched without an upstream'); +}); + +test('the tracking branch and the remote it fetches from are printed', async (t) => { + const app = await fixture(t); + await app.push('second commit', 'second\n'); + reportIncomingChanges(app.io); + assert.match(app.text(), /Tracking origin\/main/); + assert.match(app.text(), new RegExp(`Fetching from ${literal(app.origin)}`)); +}); + +test('the incoming commits and their diffstat are printed, and the report alone applies nothing', async (t) => { + const app = await fixture(t); + const before = git(app.clone, ['rev-parse', 'HEAD']); + await app.push('second commit', 'second\n'); + await app.push('third commit', 'third\n'); + const plan = reportIncomingChanges(app.io); + const text = app.text(); + assert.match(text, /2 incoming commit\(s\)/); + assert.match(text, /second commit/); + assert.match(text, /third commit/); + assert.match(text, /README\.md \| 2 \+-/); + assert.equal(plan.fallback, false); + assert.equal( + git(app.clone, ['rev-parse', 'HEAD']), + before, + 'the report must not move the checkout', + ); +}); + +test('the revision that is applied is the revision that was disclosed', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + const plan = reportIncomingChanges(app.io); + // Something lands on the remote between the disclosure and the apply. A + // second fetch would install it without ever naming it. + await app.push('a commit nobody was shown', 'fourth\n'); + assert.equal(plan.apply, disclosed); + app.io.apply(['merge', '--ff-only', plan.apply]); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), disclosed); + assert.match(app.text(), new RegExp(`Applying ${disclosed}`)); +}); + +test('the whole update applies exactly one disclosed revision', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + const plan = updateFromRemote(app.io); + assert.equal(plan.apply, disclosed); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), disclosed); +}); + +test('a remote that moves during the update does not change what is applied', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + let later = null; + const plan = updateFromRemote({ + ...app.io, + fetchRemote: (remote) => { + git(app.clone, ['fetch', '--quiet', remote]); + // The remote gains a commit after the one fetch this update makes. Only + // a second fetch — which there must not be — could reach it. + later = app.pushSync('a commit nobody was shown', 'fourth\n'); + }, + }); + assert.equal(plan.apply, disclosed); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), disclosed); + assert.notEqual(git(app.clone, ['rev-parse', 'HEAD']), later); +}); + +test('an up-to-date checkout says so and applies nothing', async (t) => { + const app = await fixture(t); + const before = git(app.clone, ['rev-parse', 'HEAD']); + const plan = updateFromRemote(app.io); + assert.deepEqual(plan, { apply: null, fallback: false }); + assert.match(app.text(), /Already up to date/); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), before); +}); + +test('an unreadable commit range is reported as unreadable, not as up to date', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + const plan = reportIncomingChanges({ + ...app.io, + read: (args) => + args[0] === 'log' ? null : readGit(args, { cwd: app.clone }), + }); + const text = app.text(); + assert.doesNotMatch(text, /Already up to date/); + assert.match(text, /Could not list the commits/); + assert.equal(plan.apply, disclosed, 'the fetched revision is still applied'); +}); + +test('an upstream that cannot be resolved after fetching stops the update', async (t) => { + const app = await fixture(t); + const before = git(app.clone, ['rev-parse', 'HEAD']); + await app.push('second commit', 'second\n'); + const plan = updateFromRemote({ + ...app.io, + // The target ref resolves to nothing after the fetch — a tracking ref + // pruned out from under the run. + read: (args) => + args[0] === 'rev-parse' && + args.length === 2 && + args[1].startsWith('refs/') + ? null + : readGit(args, { cwd: app.clone }), + }); + assert.deepEqual(plan, { apply: null, fallback: false, stopped: true }); + assert.doesNotMatch(app.text(), /Already up to date/); + assert.match(app.text(), /Could not resolve origin\/main after fetching/); + assert.match(app.text(), /Stopping/); + // Nothing may be applied: a fallback pull here would fetch again and could + // install a revision this run never resolved. + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), before); +}); + +test('a remote whose name contains a slash is read correctly', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + git(app.clone, ['remote', 'rename', 'origin', 'team/origin']); + git(app.clone, ['fetch', '--quiet', 'team/origin']); + git(app.clone, ['branch', '--set-upstream-to=team/origin/main', 'main']); + const plan = updateFromRemote({ + ...app.io, + fetchRemote: (remote) => { + assert.equal(remote, 'team/origin'); + git(app.clone, ['fetch', '--quiet', remote]); + }, + }); + assert.equal(plan.apply, disclosed); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), disclosed); + assert.match(app.text(), new RegExp(`Fetching from ${literal(app.origin)}`)); +}); + +test('a diffstat that cannot be read is reported, and the commits still are', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + const plan = reportIncomingChanges({ + ...app.io, + read: (args) => + args[0] === 'diff' ? null : readGit(args, { cwd: app.clone }), + }); + assert.equal(plan.apply, disclosed); + assert.match(app.text(), /1 incoming commit\(s\)/); + assert.match(app.text(), /Could not read the diffstat/); + assert.doesNotMatch(app.text(), /Files affected/); +}); + +test('diverged history is refused rather than applied', async (t) => { + const app = await fixture(t); + await app.push('second commit', 'second\n'); + await writeFile(path.join(app.clone, 'README.md'), 'local work\n'); + git(app.clone, ['add', 'README.md']); + git(app.clone, ['commit', '--quiet', '-m', 'local commit']); + const before = git(app.clone, ['rev-parse', 'HEAD']); + const plan = reportIncomingChanges(app.io); + const refused = spawnSync('git', ['merge', '--ff-only', plan.apply], { + cwd: app.clone, + encoding: 'utf8', + }); + assert.notEqual(refused.status, 0, 'a fast-forward-only merge must refuse'); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), before); +}); + +test('a remote that is behind this checkout is named, not announced as incoming', async (t) => { + const app = await fixture(t); + await writeFile(path.join(app.clone, 'README.md'), 'ahead\n'); + git(app.clone, ['add', 'README.md']); + git(app.clone, ['commit', '--quiet', '-m', 'local commit']); + const plan = reportIncomingChanges(app.io); + assert.match(app.text(), /is not ahead of this checkout/); + assert.doesNotMatch(app.text(), /incoming commit/); + assert.ok(plan.apply, 'the fetched revision is still named'); +}); + +test('a stop means nothing is installed, and the script fails', async (t) => { + const app = await fixture(t); + const before = git(app.clone, ['rev-parse', 'HEAD']); + await app.push('second commit', 'second\n'); + const installs = []; + const failures = []; + const plan = runPinokioUpdate({ + ...app.io, + read: (args) => + args[0] === 'rev-parse' && + args.length === 2 && + args[1].startsWith('refs/') + ? null + : readGit(args, { cwd: app.clone }), + install: () => installs.push('install'), + fail: (code) => failures.push(code), + }); + assert.equal(plan.stopped, true); + // Installing here would run install scripts and report success for an update + // that never happened. + assert.deepEqual(installs, []); + assert.deepEqual(failures, [1]); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), before); +}); + +test('an ordinary update installs once and does not fail', async (t) => { + const app = await fixture(t); + const disclosed = await app.push('second commit', 'second\n'); + const installs = []; + const failures = []; + const plan = runPinokioUpdate({ + ...app.io, + install: () => installs.push('install'), + fail: (code) => failures.push(code), + }); + assert.equal(plan.apply, disclosed); + assert.equal(git(app.clone, ['rev-parse', 'HEAD']), disclosed); + assert.deepEqual(installs, ['install']); + assert.deepEqual(failures, []); +}); + +test('an up-to-date checkout still reinstalls', async (t) => { + const app = await fixture(t); + const installs = []; + const plan = runPinokioUpdate({ + ...app.io, + install: () => installs.push('install'), + fail: () => assert.fail('an up-to-date checkout is not a failure'), + }); + assert.deepEqual(plan, { apply: null, fallback: false }); + assert.deepEqual(installs, ['install']); +}); + +test('the entrypoint is what the direct invocation runs', () => { + const guarded = source.slice(source.indexOf('if (isDirectInvocation(')); + assert.match(guarded, /runPinokioUpdate\(\);/); + // The install must not sit beside it in the guard, where a stop could not + // prevent it. + assert.doesNotMatch(guarded, /installPinokioDependencies\(\)/); +}); + +test('a password in the remote URL is not printed', () => { + assert.equal( + redactRemoteUrl('https://someone:ghp_secret@github.com/org/repo.git'), + 'https://***@github.com/org/repo.git', + ); +}); + +test('a bare token in the remote URL is not printed', () => { + const redacted = redactRemoteUrl( + 'https://ghp_secret@github.com/org/repo.git', + ); + assert.doesNotMatch(redacted, /ghp_secret/); + assert.match(redacted, /github\.com\/org\/repo\.git/); +}); + +test('a credential in a query parameter is not printed', () => { + assert.equal( + redactRemoteUrl('https://host.example/org/repo.git?access_token=secret'), + 'https://host.example/org/repo.git', + ); + // The whole query goes, so a parameter name nobody thought of goes with it. + assert.equal( + redactRemoteUrl('https://host.example/org/repo.git?sneaky=secret&x=1#frag'), + 'https://host.example/org/repo.git', + ); + assert.equal( + redactRemoteUrl('https://user:pw@host.example/org/repo.git?token=secret'), + 'https://***@host.example/org/repo.git', + ); +}); + +test('an ordinary remote URL is printed unchanged', () => { + const url = 'https://github.com/bilawalsidhu/gods-eye-view.git'; + assert.equal(redactRemoteUrl(url), url); + assert.equal( + redactRemoteUrl('git@github.com:bilawalsidhu/gods-eye-view.git'), + 'git@github.com:bilawalsidhu/gods-eye-view.git', + ); + assert.equal(redactRemoteUrl(null), null); + assert.equal(redactRemoteUrl(''), null); +}); + +test('a credential-bearing remote is redacted in the printed report', async (t) => { + const app = await fixture(t); + await app.push('second commit', 'second\n'); + reportIncomingChanges({ + ...app.io, + read: (args) => + args[0] === 'remote' + ? 'https://someone:ghp_secret@github.com/org/repo.git' + : readGit(args, { cwd: app.clone }), + }); + assert.doesNotMatch(app.text(), /ghp_secret/); + assert.match( + app.text(), + /Fetching from https:\/\/\*\*\*@github\.com\/org\/repo\.git/, + ); +}); + +test('a failed git read is null, and an empty successful read is an empty string', async (t) => { + const app = await fixture(t); + assert.equal( + readGit(['rev-parse', '--verify', 'refs/heads/absent'], { cwd: app.clone }), + null, + ); + assert.equal( + readGit(['log', '--oneline', 'HEAD..HEAD'], { cwd: app.clone }), + '', + ); +}); + +test('the update and reinstall stay behind the direct-invocation guard', () => { + // Without the guard, importing this module to test it would update the + // checkout and run npm ci as an import side effect. + assert.match( + source, + /if \(isDirectInvocation\(process\.argv\[1\], MODULE_PATH\)\) \{/, + ); + const entrypoint = source.slice( + source.indexOf('export function runPinokioUpdate'), + ); + // Order is the whole point of the fix: disclose and apply, then install. + assert.ok( + entrypoint.indexOf('updateFromRemote(io)') < + entrypoint.indexOf('install()'), + 'the update must be disclosed and applied before dependencies are installed', + ); +}); + +test('the disclosed revision is applied by merge, not by a second pull', () => { + const applier = source.slice( + source.indexOf('export function updateFromRemote'), + ); + assert.match(applier, /apply\(\['merge', '--ff-only', plan\.apply\]\)/); + // The plain pull remains, but only on the path where nothing was disclosed. + assert.match( + applier, + /else if \(plan\.fallback\) apply\(\['pull', '--ff-only'\]\)/, + ); +});