Two low-severity findings left from the #297 review: async callback and unsanitised artifact path

[wormeyman]

Reviewing #297 turned up 15 findings. Thirteen were fixed in that PR (see
3004ed6). These two were graded low and deliberately left, and they are
recorded here rather than dropped so the decision is visible. Neither is a bug
today; both are shapes that fail silently rather than loudly, in machinery that
only runs when something else is already broken.

1. withDiffArtifacts silently accepts an async callback

test/diffArtifacts.ts:343

export function withDiffArtifacts(target: DiffTarget, assertions: () => void): void

TypeScript assigns () => Promise<void> to () => void without complaint - a
void return type absorbs any returned value. So a future comparison written as

withDiffArtifacts(target, async () => {
  expect(await something()).toBeLessThan(bound);
});

compiles, returns a pending promise that is never awaited and never caught, the
try/catch sees no throw, the test passes green, and the rejection surfaces
later as an unhandled rejection attributed to a different test. No artifacts are
written either, so the one thing the wrapper exists for does not happen.

Not reachable today: all five call sites are synchronous. It becomes reachable
the first time a comparison needs await - a WASM engine handshake, or a
fixture read through fs/promises.

Options: an overload accepting and awaiting Promise<void>; or keep it
synchronous and make that enforceable rather than conventional.

2. rmSync runs on an unsanitised caller-supplied path

test/diffArtifacts.ts:316, via artifactPaths(spec, case)

rmSync(absoluteDir, { recursive: true, force: true });

dir is join("test-output", "preview-diffs", spec, case). join normalises
.. away, so a spec or case containing a traversal segment resolves outside
test-output/ and is then deleted recursively with force: true - no error, no
trace.

Every caller today passes a string literal, which is why this is low rather than
urgent. It gets less safe the moment a case name is derived from a variable,
which is the natural next step for a helper like this (one wrapped comparison
per planet, per view, per window).

A guard before the join closes the class and costs nothing on a path that only
runs after a failure:

if (!/^[\w.-]+$/.test(spec) || !/^[\w.-]+$/.test(caseName)) throw new Error(...)

Not included

The other 13 findings are fixed. Two of them were not on the original list and
were taken while editing the same functions: toRgb now rejects a buffer
shorter than its declared size, and the error.message assignment is inside a
try/catch so a diagnostic can never replace a real assertion failure.

One thing worth knowing for whoever picks this up: the decodePng CRC check
added in #297 is guarded by a planted flipped byte and a corrupted payload in
test/diffArtifacts.spec.ts. Before that, encodePng.ts's header claimed the
round-trip caught a wrong CRC and the check did not exist - so if you touch
either file, keep the planted break rather than trusting the comment.