diff --git a/tests/unit/cli/studio.test.ts b/tests/unit/cli/studio.test.ts index 39e43d45..c02a48f2 100644 --- a/tests/unit/cli/studio.test.ts +++ b/tests/unit/cli/studio.test.ts @@ -45,6 +45,7 @@ import { MarkStore } from '../../../src/studio/mark/store.js'; import { ProfileStore } from '../../../src/studio/profile-store.js'; import { scopeStorageStateToOrigin } from '../../../src/studio/login-capture.js'; import { readFileSync, mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { EventEmitter } from 'node:events'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { SUBSTRATE_RECORD } from '../../../src/studio/substrate-acquire.js'; @@ -1720,6 +1721,80 @@ describe('runStudio — the acquired substrate is the only launch target', () => }); }); +/** + * THE ASYNCHRONOUS SPAWN FAILURE, which is a different failure from the try/catch already pinned + * above. + * + * `spawn()` does NOT throw for EACCES, EPERM or a Gatekeeper refusal — it returns a child and + * reports the failure by emitting `'error'` on a later tick, so `runStudio`'s try/catch, which can + * only ever see a synchronous throw, is not in that path at all. An `'error'` event with no + * listener is rethrown by `EventEmitter` as an uncaught exception, and the child is `unref`'d and + * detached by then, so the exception lands on a CLI process that has already returned to the + * human's shell. + * + * `runStudio` has attached that listener since it was written; nothing asserted it. `fakeSpawn` + * returns `{ on: () => undefined }`, so deleting the listener leaves all six arms above green + * while the process-killing shape reopens. `defaultLaunch` carries this exact pin in + * tests/unit/studio/auto-launch.test.ts, and the asymmetry was the gap. + * + * THE FAKE IS A REAL `EventEmitter` ON PURPOSE. The throw it produces with no listener attached is + * Node's own behaviour rather than a simulation of it, which is what lets `rethrown` stand in for + * "the CLI process would have died here" without actually killing the runner. + */ +describe('runStudio — a spawn that fails asynchronously must not kill the process', () => { + let dataDir: string; + + beforeEach(() => { + dataDir = mkdtempSync(join(tmpdir(), 'wig-runstudio-err-')); + }); + + afterEach(() => { + rmSync(dataDir, { recursive: true, force: true }); + }); + + /** A stand-in child that records the state of its own listeners at `unref()` time. */ + function fakeChild(order: string[]): EventEmitter & { unref(): void } { + const child = new EventEmitter() as EventEmitter & { unref(): void }; + child.unref = () => order.push(`unref:errorListeners=${child.listenerCount('error')}`); + return child; + } + + it('attaches the error listener BEFORE unref, so there is no window', () => { + plantRecord(dataDir); + const order: string[] = []; + const child = fakeChild(order); + + runStudio([], { dataDir, spawnFn: () => child, log: () => undefined }); + + // Not merely "a listener exists by the time the test looks" — it existed at the one moment the + // command hands the child away, detaches it and stops being able to attach anything. + expect(order).toEqual(['unref:errorListeners=1']); + }); + + it('reports the failure on the human line instead of rethrowing it as an uncaught exception', () => { + plantRecord(dataDir); + const lines: string[] = []; + const child = fakeChild([]); + + runStudio([], { dataDir, spawnFn: () => child, log: (m) => lines.push(m) }); + + // Emitted only now, and that ordering IS the defect being pinned: the failure arrives after + // `runStudio` has returned, so nothing on the synchronous path is still holding a catch for it. + let rethrown: unknown = null; + try { + child.emit('error', new Error('spawn EACCES')); + } catch (e) { + rethrown = e; + } + expect(rethrown).toBeNull(); + // …and it is not swallowed either: a human typed the command, so the reason the window never + // appeared has to reach them rather than vanishing with the detached child. + expect(lines.join('\n')).toMatch(/spawn EACCES/); + // Capability language holds on the failure line too — it is user-facing text. + expect(lines.join('\n')).not.toMatch(/electron|playwright|chromium/i); + }); +}); + describe('runStudio — the deleted workspace leaves no residue in the source', () => { it('src/cli/studio.ts references no apps/ workspace and no -w flag', () => { // The split left ZERO workspace surface: there is no `workspaces` key and no `apps/` on this diff --git a/tests/unit/studio/substrate-acquire.test.ts b/tests/unit/studio/substrate-acquire.test.ts index 655b2fee..9a832fca 100644 --- a/tests/unit/studio/substrate-acquire.test.ts +++ b/tests/unit/studio/substrate-acquire.test.ts @@ -13,6 +13,7 @@ import { substratePresent, substrateRoot, SUBSTRATE_PATH_ENV, + SUBSTRATE_RECORD, type SubstrateSource, } from '../../../src/studio/substrate-acquire.js'; @@ -283,6 +284,71 @@ describe('the version a record is filed under must be one directory name', () => expect(r.outcome).toBe('acquired'); expect(readSubstrateRecord(dataDir)?.version).toBe('1.2.3'); }); + + /** + * THE TWO VERSIONS THAT CARRY NO SEPARATOR AND STILL NAME SOMEWHERE ELSE. + * + * The arm above plants `../escape`, so it stays green against a predicate narrowed to + * `!/[\\/]/` — and that narrowing looks like a simplification, because "one directory name" and + * "contains no separator" read as the same rule. They are not. `destDir` is `join(root, version)` + * and the NEXT statement is `rmSync(destDir, { recursive: true, force: true })`, so: + * + * `.` → destDir IS the substrate root → deletes every installed version and the record + * `..` → destDir IS the data dir → deletes the cache DB, the keys and the profiles + * + * Neither spelling contains a separator, so neither is caught by the narrowed form, and both + * reach a recursive delete before anything is verified. + * + * WHY THE FIXTURE HALF-UNINSTALLS FIRST. With a valid record on disk `acquireSubstrate` returns + * `already_present` before it ever computes `destDir`, so a fully-healthy machine cannot reach + * the delete at all. The state that CAN is the one this file already names elsewhere — the + * record is still there and the executable it points at is gone — which is what an interrupted + * uninstall, a partial upgrade, or a first run against a populated root all look like. + * + * The assertions are about what survived, not about the outcome word, because `..` under the + * narrowed predicate still ends in `failed`: it destroys the data dir, then fails writing the + * record into the `substrate/` directory it just deleted. An outcome-only arm would go green on + * the very shape that wiped the machine. + */ + it.each(['.', '..'])('refuses version %j rather than making it the directory it deletes', async (version) => { + await acquireSubstrate({ dataDir, source: localPathSource(sourceDir) }); + const root = substrateRoot(dataDir); + const installed = join(root, '1.2.3'); + const exec = join(installed, 'bin', 'run'); + expect(readSubstrateRecord(dataDir)?.version).toBe('1.2.3'); + + // A data-dir neighbour that only `..` reaches — the cache DB stands in for everything the + // substrate root's PARENT holds and that acquisition has no business touching. + const neighbour = join(dataDir, 'cache.db'); + writeFileSync(neighbour, 'not-a-substrate'); + + // Half-uninstalled: the record survives, its executable does not, so the record reads as + // absent and the acquisition proceeds past the `already_present` gate. + rmSync(exec); + expect(readSubstrateRecord(dataDir)).toBeNull(); + + const evil: SubstrateSource = { + id: 'evil', + manifest: { version, executable: 'bin/run' }, + async install(destDir: string) { + mkdirSync(join(destDir, 'bin'), { recursive: true }); + writeFileSync(join(destDir, 'bin', 'run'), '#!/bin/sh\n'); + }, + }; + const r = await acquireSubstrate({ dataDir, source: evil }); + + expect(r.outcome).toBe('failed'); + // Nothing was deleted: the earlier install's directory, the record filed beside it, and the + // rest of the data dir are all still where they were. + expect(existsSync(installed)).toBe(true); + expect(existsSync(join(root, SUBSTRATE_RECORD))).toBe(true); + expect(existsSync(neighbour)).toBe(true); + expect(readFileSync(neighbour, 'utf-8')).toBe('not-a-substrate'); + // And the earlier record reads back the moment its executable returns — only possible because + // neither the record nor the directory it names was removed. + writeFileSync(exec, '#!/bin/sh\n'); + expect(readSubstrateRecord(dataDir)?.version).toBe('1.2.3'); + }); }); /** @@ -492,6 +558,59 @@ describe('the install refuses a tree whose links leave it', () => { }); }); +/** + * THE WALK TERMINATES, AND ONLY ONE LINE MAKES THAT TRUE. + * + * `findEscapingLink` walks the installed tree with `readdirSync(withFileTypes)`, which stats + * WITHOUT following, so a link to a directory reports `isSymbolicLink()` and never + * `isDirectory()` — it is judged as a link and is not pushed onto the queue. That single fact is + * the whole termination argument, and the arms above do not test it: the framework fixture has + * contained directory links, but none of them points at an ancestor, so the walk finishes for + * reasons that have nothing to do with the rule. + * + * `self -> .` is a LEGAL tree. It is contained, so it must install; a bundle can legitimately + * carry one (`Versions/Current -> .` shapes appear in the wild). Following directory links — + * swapping `entry.isDirectory()` for a `statSync(child).isDirectory()`, which reads as making the + * walk "more thorough" — turns it into `self/self/self/…` forever. The caller is `acquireSubstrate` + * on the warmup path, unattended and with no timeout of its own, so the failure is a warmup that + * never returns rather than a component that fails to install. + * + * Windows is skipped for the same reason as the arms above: creating a symlink there needs + * elevation. The per-test timeout is deliberate — if the rule is ever lost, this arm must report + * as a failing test rather than as a runner that stopped making progress. + */ +describe('the walk terminates on a link cycle that is contained', () => { + it.skipIf(process.platform === 'win32')( + 'installs a tree whose directory link points at its own parent', + async () => { + const src = mkdtempSync(join(tmpdir(), 'wigolo-substrate-cycle-')); + try { + mkdirSync(join(src, 'bin'), { recursive: true }); + writeFileSync(join(src, 'bin', 'run'), '#!/bin/sh\n'); + // The cycle: a directory link back to the directory that holds it. + symlinkSync('.', join(src, 'self')); + writeFileSync(join(src, 'substrate.json'), JSON.stringify({ version: '3.3.7', executable: 'bin/run' })); + + // CONTROL: the cycle is real — the link resolves to the directory it sits in, so a walk + // that descended into it would be re-reading its own parent. + expect(realpathSync(join(src, 'self'))).toBe(realpathSync(src)); + + const r = await acquireSubstrate({ dataDir, source: localPathSource(src) }); + + // Contained is contained: this is an ACCEPT, not a refusal that happens to terminate. + expect(r.outcome).toBe('acquired'); + expect(readSubstrateRecord(dataDir)?.version).toBe('3.3.7'); + // The link survived the copy as a link, so the installed tree still carries the cycle the + // walk had to survive — not a resolved directory that quietly removed it. + expect(readlinkSync(join(substrateRoot(dataDir), '3.3.7', 'self'))).toBe('.'); + } finally { + rmSync(src, { recursive: true, force: true }); + } + }, + 10_000, + ); +}); + describe('containment is answered by the filesystem, not by string comparison', () => { let outside: string;