mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-17 07:10:12 +02:00
feat(make-pdf): out-of-tree image reads warn; --strict makes them fatal (D8.1)
Local CLI semantics stay (absolute paths and ../ still inline, like pandoc), but never silently: an agent PDF-ing untrusted markdown can't quietly embed a file from outside the input directory into a shareable document without a visible warning, and --strict pipelines hard-fail. Two unit tests. Also: TODOS.md gains the deferred e2e-harness dedup entry (D8.2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2397,3 +2397,21 @@ building once users hit multi-diagram docs; wedge perf is fine without it.
|
||||
|
||||
**Effort:** S (human ~1d, CC ~30min). **Depends on:** diagram engine wedge
|
||||
shipping (lib/diagram-render bundle versioning).
|
||||
|
||||
### P3: Dedupe the make-pdf e2e gate-test harness
|
||||
|
||||
**What:** Five e2e files (`combined-gate`, `emoji-gate`, `diagram-gate`,
|
||||
`landscape-gate`, `format-gate`) each hand-roll the same prerequisite probe
|
||||
(binary/browse/poppler checks with CI hard-fail vs local skip), mkdtemp/rm
|
||||
lifecycle, and child-timeout constants. Extract a shared
|
||||
`make-pdf/test/e2e/helpers.ts` (prerequisites(), withWorkDir(), runGenerate()).
|
||||
|
||||
**Why:** Review-army maintainability finding on v1.58.0.0 — the boilerplate
|
||||
diverges a little more with each new gate (diagram-gate now captures stderr
|
||||
via Bun.spawnSync while the others use execFileSync), and a future fix to the
|
||||
CI-hard-fail contract has to land five times.
|
||||
|
||||
**Context:** Deferred at ship time (D8.2) because it's test-only churn across
|
||||
five green files at the tail of a release. Zero user-facing value; pure DRY.
|
||||
|
||||
**Effort:** S (human ~3h, CC ~20min). **Depends on:** None.
|
||||
|
||||
@@ -619,6 +619,16 @@ export function inlineLocalImages(html: string, opts: PrepassImageOptions): stri
|
||||
const cached = memo.get(filePath);
|
||||
if (cached !== undefined) return rewriteImgTag(tag, cached);
|
||||
|
||||
// Out-of-tree reads are legal (local CLI semantics — like pandoc) but
|
||||
// never silent: an agent PDF-ing untrusted markdown should not quietly
|
||||
// embed ~/.ssh/config into a shareable document. --strict makes it fatal.
|
||||
const inputRoot = path.resolve(opts.inputDir) + path.sep;
|
||||
if (!filePath.startsWith(inputRoot)) {
|
||||
const msg = `image resolves OUTSIDE the input directory: ${src} → ${filePath}`;
|
||||
if (opts.strict) throw new StrictModeError(msg + " — move it under the markdown's directory or drop --strict");
|
||||
opts.warn(msg);
|
||||
}
|
||||
|
||||
if (!fs.existsSync(filePath)) {
|
||||
const msg = `image not found: ${src} (resolved to ${filePath})`;
|
||||
if (opts.strict) throw new StrictModeError(msg);
|
||||
|
||||
@@ -306,6 +306,35 @@ describe("inlineLocalImages", () => {
|
||||
expect(out).toContain('data-gstack-px-height="44"');
|
||||
});
|
||||
|
||||
test("out-of-tree image reads warn (never silent) and still inline", () => {
|
||||
const outside = fs.mkdtempSync(path.join(os.tmpdir(), "prepass-outside-"));
|
||||
fs.writeFileSync(path.join(outside, "ext.png"), tinyPng(10, 10));
|
||||
try {
|
||||
const warnings: string[] = [];
|
||||
const out = inlineLocalImages(`<img src="${path.join(outside, "ext.png")}">`, {
|
||||
...base, warn: (m) => warnings.push(m),
|
||||
});
|
||||
expect(out).toContain("data:image/png;base64,");
|
||||
expect(warnings.some((w) => w.includes("OUTSIDE the input directory"))).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(outside, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("out-of-tree image + --strict → StrictModeError", () => {
|
||||
const outside = fs.mkdtempSync(path.join(os.tmpdir(), "prepass-outside-"));
|
||||
fs.writeFileSync(path.join(outside, "ext.png"), tinyPng(10, 10));
|
||||
try {
|
||||
expect(() =>
|
||||
inlineLocalImages(`<img src="${path.join(outside, "ext.png")}">`, {
|
||||
...base, strict: true, warn: () => {},
|
||||
}),
|
||||
).toThrow(StrictModeError);
|
||||
} finally {
|
||||
fs.rmSync(outside, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("Windows drive-letter src is treated as a local path, not a URL scheme", () => {
|
||||
// C:/x.png matches the single-letter-scheme regex — it must reach the
|
||||
// local-path branch (and the missing-file placeholder), never silently
|
||||
@@ -313,7 +342,8 @@ describe("inlineLocalImages", () => {
|
||||
const warnings: string[] = [];
|
||||
const out = inlineLocalImages(`<img src="C:/missing/x.png">`, { ...base, warn: (m) => warnings.push(m) });
|
||||
expect(out).toContain("image-missing");
|
||||
expect(warnings.length).toBe(1);
|
||||
// Two warnings: it's out-of-tree (resolved outside inputDir) AND missing.
|
||||
expect(warnings.some((w) => w.includes("image not found"))).toBe(true);
|
||||
});
|
||||
|
||||
test("indented fences inside lists replay byte-for-byte (no list splitting)", () => {
|
||||
|
||||
Reference in New Issue
Block a user