Skip to content

docs: make Catalog visual and reproducible - #2975

Merged
ukimsanov merged 5 commits into
mainfrom
docs/catalog-presentation
Aug 4, 2026
Merged

docs: make Catalog visual and reproducible#2975
ukimsanov merged 5 commits into
mainfrom
docs/catalog-presentation

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Makes every Catalog destination useful at a glance and keeps the generated result reproducible.

  • adds preview-first presentation across all 168 current registry items
  • groups items around recognizable uses instead of implementation details
  • keeps installation and wiring instructions generated from registry manifests
  • uses the media and poster declared by each manifest instead of inventing asset URLs
  • preserves the hand-authored Catalog overview and global navigation when the generator runs

Review focus

Sample blocks and components across several categories, then rerun the generator and confirm that it produces no second-run drift.

183 of the 184 changed files are deterministic output of scripts/generate-catalog-pages.ts — the generator is the review surface (~202 changed lines).

Verification

  • generated all 168 Catalog pages
  • a second generator run reported the navigation already current
  • generator lint and formatting passed
  • Mintlify build validation passed
  • Mintlify broken-link check passed
  • no screenshots, binary media, render output, or temporary artifacts are included

Stack

Stack #2980, position 5 of 6 — base is #2974. Content is unchanged from the original submission; only the base moved, when #2973 was split into #2977#2978#2979.

Re-verified after that rebase: 0 dangling navigation entries, 0 broken internal links.

Copilot AI review requested due to automatic review settings August 3, 2026 23:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the generated HyperFrames Catalog docs so each registry item page is “preview-first” (motion preview at top), uses a consistent “Add it to a project” section (agent prompt + terminal install), and keeps generated output reproducible by deriving wiring/install details from registry-item.json manifests. It also updates the catalog preview-generation script to render items in a temp project that more closely matches post-install layout (including registry target paths).

Changes:

  • Update scripts/generate-catalog-previews.ts to copy manifest-declared files into their target locations inside the temp project before rendering.
  • Normalize many generated docs/catalog/** item pages to a consistent structure (preview, agent prompt, terminal install, installed files, usage, related links).
  • Adjust several registry manifests’ preview metadata (notably removing some poster fields).

Reviewed changes

Copilot reviewed 184 out of 184 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/generate-catalog-previews.ts Copies manifest files[].path → files[].target into the temp project so previews render with post-install paths.
registry/blocks/code-snippet-apple-terminal-solid-colors/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-silver-aerogel/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-red-sands/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-pro/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-ocean/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-novel/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-man-page/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-homebrew/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-grass/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-clear-light/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-clear-dark/registry-item.json Drops preview.poster (video-only).
registry/blocks/code-snippet-apple-terminal-basic/registry-item.json Drops preview.poster (video-only).
docs/catalog/components/yt-screen-warp.mdx Regenerated item page: preview-first + “Add it to a project” + installed files + related links.
docs/catalog/components/yt-feather-highlight.mdx Regenerated item page template.
docs/catalog/components/yt-circle-pointer.mdx Regenerated item page template.
docs/catalog/components/yt-camera-move.mdx Regenerated item page template.
docs/catalog/components/vignette.mdx Regenerated item page template.
docs/catalog/components/texture-mask-text.mdx Adds description/tags + updates install/usage sections + related links.
docs/catalog/components/shimmer-sweep.mdx Regenerated item page template.
docs/catalog/components/parallax-zoom.mdx Regenerated item page template.
docs/catalog/components/parallax-unzoom.mdx Regenerated item page template.
docs/catalog/components/morph-text.mdx Regenerated item page template.
docs/catalog/components/mk-usage-arc.mdx Regenerated item page template.
docs/catalog/components/mk-emphasis-type.mdx Regenerated item page template.
docs/catalog/components/hw-underline.mdx Regenerated item page template.
docs/catalog/components/hw-callout-circle.mdx Regenerated item page template.
docs/catalog/components/hw-box-label.mdx Regenerated item page template.
docs/catalog/components/hw-boil.mdx Regenerated item page template.
docs/catalog/components/hw-arrow.mdx Regenerated item page template.
docs/catalog/components/grid-pixelate-wipe.mdx Regenerated item page template.
docs/catalog/components/grain-overlay.mdx Regenerated item page template.
docs/catalog/components/caption-weight-shift.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-pill-karaoke.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-particle-burst.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-neon-accent.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-matrix-decode.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-highlight.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-gradient-fill.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-glitch-rgb.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-emoji-pop.mdx Updates preview src + regenerates item page template.
docs/catalog/components/caption-clip-wipe.mdx Updates preview src + regenerates item page template.
docs/catalog/blocks/yt-prism-title.mdx Regenerated block page template (preview-first, install/wiring, related links).
docs/catalog/blocks/yt-lower-third.mdx Regenerated block page template.
docs/catalog/blocks/yt-logo-intro.mdx Regenerated block page template.
docs/catalog/blocks/yt-lcd-background.mdx Regenerated block page template.
docs/catalog/blocks/yt-comment-card.mdx Regenerated block page template.
docs/catalog/blocks/x-post.mdx Regenerated block page template.
docs/catalog/blocks/world-map.mdx Regenerated block page template.
docs/catalog/blocks/whip-pan.mdx Regenerated block page template.
docs/catalog/blocks/vfx-shatter.mdx Regenerated block page template (retains warning block).
docs/catalog/blocks/vfx-portal.mdx Regenerated block page template (retains warning block).
docs/catalog/blocks/vfx-magnetic.mdx Regenerated block page template (retains warning block).
docs/catalog/blocks/vfx-liquid-glass.mdx Regenerated block page template (retains warning block).
docs/catalog/blocks/us-map.mdx Regenerated block page template.
docs/catalog/blocks/us-map-hex.mdx Regenerated block page template.
docs/catalog/blocks/us-map-flow.mdx Regenerated block page template.
docs/catalog/blocks/us-map-bubble.mdx Regenerated block page template.
docs/catalog/blocks/ui-3d-reveal.mdx Regenerated block page template.
docs/catalog/blocks/transitions-scale.mdx Regenerated block page template.
docs/catalog/blocks/transitions-radial.mdx Regenerated block page template.
docs/catalog/blocks/transitions-push.mdx Regenerated block page template.
docs/catalog/blocks/transitions-other.mdx Regenerated block page template.
docs/catalog/blocks/transitions-mechanical.mdx Regenerated block page template.
docs/catalog/blocks/transitions-light.mdx Regenerated block page template.
docs/catalog/blocks/transitions-grid.mdx Regenerated block page template.
docs/catalog/blocks/transitions-distortion.mdx Regenerated block page template.
docs/catalog/blocks/transitions-dissolve.mdx Regenerated block page template.
docs/catalog/blocks/transitions-destruction.mdx Regenerated block page template.
docs/catalog/blocks/transitions-cover.mdx Regenerated block page template.
docs/catalog/blocks/transitions-blur.mdx Regenerated block page template.
docs/catalog/blocks/transitions-3d.mdx Regenerated block page template.
docs/catalog/blocks/tiktok-follow.mdx Regenerated block page template.
docs/catalog/blocks/thermal-distortion.mdx Regenerated block page template.
docs/catalog/blocks/swirl-vortex.mdx Regenerated block page template.
docs/catalog/blocks/spotify-card.mdx Regenerated block page template.
docs/catalog/blocks/spain-map.mdx Regenerated block page template.
docs/catalog/blocks/sdf-iris.mdx Regenerated block page template.
docs/catalog/blocks/ripple-waves.mdx Regenerated block page template.
docs/catalog/blocks/ridged-burn.mdx Regenerated block page template.
docs/catalog/blocks/reddit-post.mdx Regenerated block page template.
docs/catalog/blocks/organic-light-leak-overlay.mdx Regenerated block page template.
docs/catalog/blocks/news-ticker.mdx Regenerated block page template.
docs/catalog/blocks/mk-specs-list.mdx Regenerated block page template.
docs/catalog/blocks/mk-progress-stat.mdx Regenerated block page template.
docs/catalog/blocks/mk-placeholder-grid.mdx Regenerated block page template.
docs/catalog/blocks/mk-line-graph.mdx Regenerated block page template.
docs/catalog/blocks/mk-callout-highlight.mdx Regenerated block page template.
docs/catalog/blocks/mk-background.mdx Regenerated block page template.
docs/catalog/blocks/macos-notification.mdx Regenerated block page template.
docs/catalog/blocks/lt-stack-bars.mdx Regenerated block page template.
docs/catalog/blocks/lt-soft-pill.mdx Regenerated block page template.
docs/catalog/blocks/lt-side-rule.mdx Regenerated block page template.
docs/catalog/blocks/lt-mask-reveal.mdx Regenerated block page template.
docs/catalog/blocks/lt-kicker-name.mdx Regenerated block page template.
docs/catalog/blocks/lt-dark-card.mdx Regenerated block page template.
docs/catalog/blocks/lt-color-block.mdx Regenerated block page template.
docs/catalog/blocks/lt-clean-bar.mdx Regenerated block page template.
docs/catalog/blocks/lt-bold-block.mdx Regenerated block page template.
docs/catalog/blocks/logo-outro.mdx Regenerated block page template.
docs/catalog/blocks/light-leak.mdx Regenerated block page template.
docs/catalog/blocks/instagram-follow.mdx Regenerated block page template.
docs/catalog/blocks/hw-title.mdx Regenerated block page template.
docs/catalog/blocks/hw-path-text.mdx Regenerated block page template.
docs/catalog/blocks/gravitational-lens.mdx Regenerated block page template.
docs/catalog/blocks/glitch.mdx Regenerated block page template.
docs/catalog/blocks/freeze-frame-dressing.mdx Regenerated block page template.
docs/catalog/blocks/flowchart.mdx Regenerated block page template.
docs/catalog/blocks/flowchart-vertical.mdx Regenerated block page template.
docs/catalog/blocks/flash-through-white.mdx Regenerated block page template.
docs/catalog/blocks/editorial-flash-overlay.mdx Regenerated block page template.
docs/catalog/blocks/domain-warp-dissolve.mdx Regenerated block page template.
docs/catalog/blocks/data-chart.mdx Regenerated block page template.
docs/catalog/blocks/cross-warp-morph.mdx Regenerated block page template.
docs/catalog/blocks/code-typing.mdx Regenerated block page template.
docs/catalog/blocks/code-snippet-flight.mdx Regenerated block page template.
docs/catalog/blocks/code-scroll.mdx Regenerated block page template.
docs/catalog/blocks/code-morph.mdx Regenerated block page template.
docs/catalog/blocks/code-diff.mdx Regenerated block page template.
docs/catalog/blocks/cinematic-zoom.mdx Regenerated block page template.
docs/catalog/blocks/chromatic-radial-split.mdx Regenerated block page template.
docs/catalog/blocks/camcorder-hud.mdx Regenerated block page template.
docs/catalog/blocks/beat-freeze-cut.mdx Regenerated block page template.
docs/catalog/blocks/app-showcase.mdx Regenerated block page template.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 20 to 22
"preview": {
"video": "https://static.heygen.ai/hyperframes-oss/docs/images/catalog/blocks/code-snippet-apple-terminal-solid-colors.mp4",
"poster": "https://static.heygen.ai/hyperframes-oss/docs/images/catalog/blocks/code-snippet-apple-terminal-solid-colors.png"
"video": "https://static.heygen.ai/hyperframes-oss/docs/images/catalog/blocks/code-snippet-apple-terminal-solid-colors.mp4"
}
Comment thread scripts/generate-catalog-previews.ts Outdated
Comment on lines +144 to +146
const sourcePath = join(tmpDir, file.path);
const targetPath = join(tmpDir, file.target);
if (!existsSync(sourcePath) || sourcePath === targetPath) continue;
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from 7087b7b to 0dd113f Compare August 3, 2026 23:48
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch 2 times, most recently from 2c98c4d to 75d4409 Compare August 4, 2026 00:47
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from 75d4409 to 83f21fb Compare August 4, 2026 01:52
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from 83f21fb to dfa55a4 Compare August 4, 2026 02:04
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch 2 times, most recently from 48f379e to 96b6abf Compare August 4, 2026 02:38
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from 96b6abf to 5a971e3 Compare August 4, 2026 02:48
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from 5a971e3 to bc9d249 Compare August 4, 2026 03:02
@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from bc9d249 to 270a88d Compare August 4, 2026 03:37

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one merge blocker at exact head 270a88d9.

P1 — manifest path traversal in pull-request preview CI. mirrorRegistryTargets() joins untrusted registry-item.json files[].path and files[].target directly under the temporary project and then calls cpSync. A value such as ../../home/runner/... escapes the project root; the source side can read arbitrary runner files and the target side can overwrite arbitrary paths. This executes in the pull_request Catalog Previews workflow for registry changes, and the job subsequently uploads docs/images/catalog/ artifacts.

Please resolve both source and target paths, reject any candidate whose relative(projectDir, candidate) is absolute or starts with .., and add regressions for traversal on both fields. The earlier Copilot thread identified the same primitive; it remains present at the current head despite that thread being outdated.

Everything else I checked is clean:

  • Ran generate-catalog-pages.ts twice; the checked-in 168 pages, catalog-index.json, and docs.json are byte-reproducible with no second-run drift.
  • Spot-checked posterless code-snippet-apple-terminal-basic and ordinary caption-highlight; generated content matches manifests and the #2976 doctrine.
  • Posterless video degrades to the styled neutral background until autoplay paints, without a broken-image placeholder.
  • All 13 removed/generated poster URLs return 403; all corresponding video URLs return 200.
  • Targeted formatting/lint checks pass.

@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Confirmed and fixed at 48f6bdd4. This one is real and it is mine — I introduced the wrapper when I extracted this block for a complexity finding earlier in the stack, and never looked at what it was joining.

Why it's exploitable, stated plainly for the record. catalog-previews.yml triggers on pull_request for registry/blocks/** and registry/components/**, so the manifest arrives from the pull request. join() walks out of its first argument, so a files[].path of ../../../../etc/passwd reads an arbitrary runner file into the project — which the job then uploads as an artifact — and a files[].target of the same shape writes an arbitrary runner path. You were right to check both fields rather than just the one that looks like input.

The fix. Both sides resolved, rejected when relative(projectDir, candidate) is absolute or starts with .., exactly as you specified. Traversal that lands back inside the project still works, so nested/../demo.html is unaffected.

Containment is in scripts/registry-target-paths.mjs rather than inline, for a reason worth naming: importing generate-catalog-previews.ts in a test drags in the producer, so the traversal cases would not have been testable where the code was. existsSync is injected so the decision can't depend on whether the target happens to exist on the runner — otherwise the traversal test passes for the wrong reason.

Eight tests: traversal on each field separately, absolute paths on each field, the sibling directory that shares the project's prefix (../hf-catalog-demo-evil), traversal that returns inside, and incomplete entries. Wired into bun run test:scripts.

Verified end to end, not only in unit tests. On a real tree, a manifest asking to read ../secret.txt and write ../pwned.txt produces neither file, while the legitimate entry still copies. I wanted the on-disk result rather than an assertion about my own helper.

On the Copilot thread. You're right that it identified the same primitive and that it survived to head despite being marked outdated. Worth saying explicitly: it was outdated because the code moved, not because it was fixed. That's a failure mode I'll watch for — a resolved thread on relocated code reads exactly like a fixed one.

Thanks for the rest of the pass too. The double generator run for byte-reproducibility and the 403/200 check across all 13 poster and video URLs are the two things I'd most want verified here and I hadn't done either at that depth.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at exact head 48f6bdd47: CHANGES_REQUESTED. The original .. traversal is closed, but the containment check is still lexical and remains bypassable through a symlink inside the copied registry item.

P1 — a symlinked target still writes outside the preview project. isContainedIn(root, candidate) only compares resolve()/ relative(); those do not resolve filesystem symlinks. Registry items are copied recursively into the temp project with symlinks preserved. A PR can add escape -> /tmp/outside and declare target: "escape/pwned.txt". The helper returns true, mkdirSync(dirname(to), {recursive:true}) follows the link, and cpSync(from,to) writes outside the project.

I reproduced the exact shape locally: the new predicate returned allowed: true, and the copy created /tmp/.../outside/pwned.txt with the source contents. The same class needs considering on the source side too.

Please make containment filesystem-aware (for example, reject symlinks in every existing path component and verify real paths/real parents against a real project root before copying), and add source + target symlink-escape regressions. The current eight tests only exercise string traversal/absolute paths, so they stay green under this bypass.

Everything from my first review still holds otherwise: page generation was deterministic across two full runs, the two spot-checked pages matched their manifests, and posterless videos degrade sensibly.

ukimsanov added a commit that referenced this pull request Aug 4, 2026
Miguel's second P1 on #2975, and he is right that my first fix only closed half
of it. `resolve()` and `relative()` are string operations and do not follow
links. Registry items are copied in recursively with symlinks preserved, so a
PR shipping `escape -> /tmp/outside` and declaring `target: "escape/pwned.txt"`
passed the lexical check, `mkdirSync` followed the link, and `cpSync` wrote
outside the project.

Reproduced before fixing: the old predicate returned one allowed copy and the
file appeared outside the project. Both directions were exposed — a symlinked
`path` reads a runner file in just as readily.

Containment is now filesystem-aware. No existing component of a candidate may
be a symlink, and the candidate's real location — resolved through its deepest
existing ancestor — has to sit under the project's own real path. A symlink is
refused rather than followed, even one pointing back inside the project:
nothing in the registry needs one, and following it would mean trusting the
target not to change between the check and the copy.

The tests are real fixtures now instead of string cases, because a purely
lexical suite is exactly what stayed green through the bypass. Twelve of them,
covering a symlinked target directory, a symlinked source file, a deeper path
through a symlinked component, an inward-pointing symlink, plus the lexical and
absolute cases from before.
@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Fixed at 82e78fb3. You were right that the first fix only closed half of it, and I reproduced the bypass before touching anything: the old predicate returned one allowed copy and pwned.txt appeared outside the project. Both directions were exposed — a symlinked path reads a runner file in just as readily as a symlinked target writes one out.

Containment is filesystem-aware now. No existing component of a candidate may be a symlink, and the candidate's real location — resolved through its deepest existing ancestor — has to sit under the project's own real path. Lexical containment still runs first as a cheap reject.

One decision worth flagging since it is stricter than what you asked for: a symlink is refused rather than followed, even one pointing back inside the project. Nothing in the registry needs one, and resolving-then-allowing would mean trusting the link target not to change between the check and the copy. If a legitimate registry item ever needs a symlink, that should be a deliberate allowance rather than a hole that was already open.

The tests are real fixtures now, not string cases. That was the actual lesson here — the previous eight tests were lexical, so every symlink case passed them, which is exactly how the bypass stayed green. Twelve now: symlinked target directory, symlinked source file, a deeper path through a symlinked component (escape/a/b/c.txt), an inward-pointing symlink, plus the lexical and absolute cases from before.

Verified on disk again rather than only through assertions: with escape -> /tmp/outside and leak.txt -> ../secret.txt present in the fixture project, neither outside/pwned.txt nor stolen.txt is created, and the legitimate entry still copies.

Two passes, two real escapes, both found by you. The first one I inherited and did not look at; the second I wrote. Thanks for not stopping at the lexical fix.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head 82e78fb357a20f92e7bab831a0ca0f382a7fc233. The head moved twice while I was working, so this is written against the latest — including fix(scripts): make registry path containment filesystem-aware, which landed after the traversal issue was raised.

You asked me to review the generator, spot-check two generated pages, and skip the other 181. I did the first part differently — regenerating rather than sampling — and the reason turns out to matter.

The generated tree is provably clean, so "skip the rest" is safe to say

Rather than spot-check, I ran the generator at this head in a clean worktree:

npx tsx scripts/generate-catalog-pages.ts
git status --porcelain   # zero lines

All 168 catalog pages, docs/public/catalog-index.json, and the catalog groups in docs.json reproduce byte-for-byte from registry/. Nothing hand-edited is hiding in that part of the tree, so a reviewer really can skip those files — which two spot-checks could not have established. I re-ran it after the latest push and it is still clean.

The generator reads well. It drives off registry/registry.json instead of walking directories, maps only hyperframes:block and hyperframes:component and continues on anything else — which is why registry/examples/ correctly produces no pages — and the count lands at exactly 132 + 36 = 168.

Where the scope in your message no longer matches the PR

At this head the PR is 187 files, and 17 of them are hand-written, not one:

docs/catalog/** + docs/public/catalog-index.json + docs/docs.json 170, generated, verified reproducible
registry/**/registry-item.json 12, hand-edited (the poster removals)
scripts/generate-catalog-pages.ts, scripts/generate-catalog-previews.ts 2, hand-edited
scripts/registry-target-paths.mjs, scripts/registry-target-paths.test.mjs, package.json 3, hand-edited

That last row is entirely new since your message — a 92-line containment module, a 12-case test suite, and a test:scripts registration, across two commits. It is the security-relevant part of the PR, and "review the generator and skip the other 181" now routes a reviewer straight past it. No criticism of the instruction, which was accurate when written; flagging it because the same wording will be read by whoever reviews next.

The containment fix holds, including cases beyond the reported one

The traversal issue was raised against the earlier lexical version, and I confirmed it reproduced there before the fix landed. At this head I re-tested the new filesystem-aware version directly rather than reading the regexes, with a real fixture tree and seven vectors:

Case Result
ordinary demo.htmlout/demo.html allowed
lexical ../../../pwned.txt target refused
absolute /tmp/pwned.txt target refused
target through a symlinked directory in the project refused
target through a symlink nested in an intermediate component (sub/deep/…) refused
symlink pointing back inside the project refused
symlink used as the read source refused

Nothing landed outside the project root in any case. The last three are the ones worth naming: an intermediate-component symlink is the version that survives a naive "check the final path" fix, refusing an inward-pointing link closes the check-then-copy window rather than trusting the link target to stay put, and guarding the source side matters because that is the direction that pulls a runner file into an uploaded artifact. The docstring is explicit about all three rather than leaving them to be inferred, which is the right call for a file whose whole job is a security invariant.

The test suite is an independent assertion set rather than a re-implementation of the guard, and still includes the prefix-sibling case (../hf-catalog-demo-evil/x) that a naive startsWith(root) waves through. 12/12 pass locally. The threat model in the docstring also checks out at source: catalog-previews.yml does trigger on pull_request for registry/** changes and does upload docs/images/catalog/ via actions/upload-artifact.

One character is holding up most of the red columns

Format and Preflight (lint + format) are failing at this head, and it is a formatting nit in the new file, not a logic problem. Reproduced locally:

npx oxfmt --check scripts/registry-target-paths.mjs
→ Format issues found

scripts/registry-target-paths.mjs:39 — oxfmt wants a space before the closing paren of the empty for-update clause:

-  for (let current = candidate; current !== root && isBeneath(root, current);) {
+  for (let current = candidate; current !== root && isBeneath(root, current); ) {

npx oxfmt scripts/registry-target-paths.mjs fixes it. Worth doing promptly rather than at leisure, because of what it gates: in catalog-previews.yml, render-previews declares needs: preflight. While Preflight is red, the catalog preview render never runs — so the one job that would exercise the containment module end to end is skipped, on the PR that introduces it.

Related and still open from before: scripts/registry-target-paths.mjs is not in that workflow's paths filter (registry/blocks/**, registry/components/**, scripts/generate-catalog-previews.ts, and the workflow itself). A future change to the containment module alone will not re-run the preview job. The unit tests do run — bun run test:scripts is wired at .github/workflows/ci.yml:279 — but the integration path stays unexercised. Adding one line to the filter closes it.

The rest of the red is not evidence of anything: regression, preview-regression, and player-perf each failed in about 4 seconds on a results-aggregation step with their shards cancelled, so nothing rendered. Required Test is still running; Build, Typecheck, Test: runtime contract, and Semantic PR title are green.

Posters: the removal set is exactly the broken set

I checked the reason and the boundary separately, since a correct removal for a wrong reason still misleads the next edit.

  • All 12 removed poster URLs return 403. Your reason holds.
  • All 154 poster URLs still emitted across the generated catalog return 200, and all 167 <video src> URLs return 200. The removal is not just correct, it is complete — no thirteenth item is quietly pointing at a dead poster, and no video source is broken.

(Minor: the diff removes 12 poster fields, not 13. Fourteen generated pages now emit no poster attribute — the 12 plus morph-text and texture-mask-text, which already had none.)

Degradation is fine. generate-catalog-pages.ts:358-361 omits the attribute entirely rather than emitting poster="", and the element keeps className="… bg-zinc-100 dark:bg-zinc-800" with autoPlay muted loop playsInline. A posterless card shows a theme-appropriate neutral panel in both light and dark for the instant before the first frame decodes — and since these autoplay muted, that instant is all a poster was ever covering.

Where I land

Everything I set out to verify came back clean, and the traversal fix is genuinely thorough — it closes vectors past the one that prompted it. Holding at comment rather than approving only because Format is red and Preflight gates the preview render; fix the one line, let Preflight and Test come back, and I am happy to approve at that head.

Review by Rames Jusso

@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch 2 times, most recently from 6ae3b49 to 923e77e Compare August 4, 2026 04:31

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 923e77e7f492ad2247f01a69069c43561ae8346c. I said last round that I would approve once the format fix landed and Preflight and Test came back. They have, and the thing I actually cared about happened as a result. Approving.

The one character, and what it unblocked

The only change to the containment module since the head I tested is the space:

-  for (let current = candidate; current !== root && isBeneath(root, current);) {
+  for (let current = candidate; current !== root && isBeneath(root, current); ) {

Confirmed with the repo-pinned formatter rather than a floating one — ./node_modules/.bin/oxfmt --version reports 0.41.0, matching the ^0.41.0 devDependency, and --check on the file passes. That is consistent with your read that the disagreement was a newer binary resolved outside the project.

The reason it was worth doing promptly rather than at leisure: Render catalog previews ran and passed at this head. It declares needs: preflight, so for as long as Preflight was red that job never started — the one job that exercises the new containment module end to end was being skipped on the PR that introduces it. It is green now, so the module has actually been through the workflow it was written for.

Re-verified rather than carried forward

  • Containment, 8 vectors, real fixture tree. Ordinary file allowed. Refused: lexical ../../../pwned.txt, absolute /tmp/pwned.txt, a directory symlink pointing outside, a symlink nested in an intermediate component (sub/deep/…), a symlink pointing back inside the project, the prefix sibling ../project-evil/x, and a symlink used as the read source rather than the write target. Nothing landed outside the project root.
  • The module's own suite: 12/12.
  • Generator determinism: npx tsx scripts/generate-catalog-pages.ts then git status --porcelain → zero lines. 168 items, docs.json already current. Nothing hand-edited is hiding in the generated tree at this head either.

On refusing an inward-pointing symlink rather than resolving and allowing it — that is stricter than what I raised, and it is the right call for the reason the docstring gives. Resolving then allowing means trusting the link target not to change between the check and the copy, and nothing in the registry needs a link in the first place. Cheapest correct thing.

Still open, not blocking

.github/workflows/catalog-previews.yml triggers on registry/blocks/**, registry/components/**, scripts/generate-catalog-previews.ts, and itself. scripts/registry-target-paths.mjs is still not in that list. It does not matter for this PR — the manifest edits match registry/**, which is why the preview job ran — but a future change to the containment module alone will not re-run the job that exercises it. The unit tests still run via test:scripts; it is the integration path that goes quiet. One line.

CI

Seven of the eight required contexts are green at this head; regression has not reported yet, with shards still running. Zero failure conclusions among any check on the commit.

Approving on my own verification, per the condition I stated last round. The approval is a statement about 923e77e7 specifically — this repo requires an approval at or after the last push, so another push retires it. It also does not clear the separate open change-request on this PR; that one is not mine to close.

Review by Rames Jusso

@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch 2 times, most recently from 1b515fe to fdde883 Compare August 4, 2026 05:02
@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Paths filter fixed at fdde8836scripts/registry-target-paths.mjs is in catalog-previews.yml now.

Worth naming why I took a non-blocking note straight away: it is the same shape as the bug the module exists to prevent. The defence is present, and the only job that exercises it was not wired to run when the defence itself changes. A future contributor tightening or loosening containment would have gotten a green PR with the relevant job never scheduled.

Your needs: preflight observation from the round before was the other half of that, and the more alarming half — while Preflight was red on my formatting, Render catalog previews was being skipped on the exact PR that introduces the containment module. Two independent ways for that job to silently not run, on the one PR where it matters most.

@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from fdde883 to f2be325 Compare August 4, 2026 05:15

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head f2be325b. Re-approving, and my last open item on this PR is closed.

The paths filter

- "scripts/generate-catalog-previews.ts"
# The containment module the renderer imports. Without it a change to
# path-traversal defence alone never re-runs the job that exercises it.
- "scripts/registry-target-paths.mjs"

That is the gap closed, and the comment is the part I would not have asked for but is worth more than the line itself — the next person tempted to tidy the filter now knows why that entry exists.

Re-verified rather than assumed

The containment module and its test file are byte-identical to the head I verified last round (git diff over both paths is empty), so the traversal analysis carries. I re-ran everything anyway since the tree moved underneath it:

  • 8 traversal vectors against a real fixture tree — ordinary file allowed; lexical ../, absolute path, symlinked directory, symlink nested in an intermediate component, inward-pointing symlink, prefix sibling, and symlink-as-read-source all refused.
  • Module suite: 12/12.
  • Generator determinism: regenerate then git status --porcelain → zero lines, 168 pages, docs.json already current. Still nothing hand-edited hiding in the generated tree.

Everything else in this PR's file diff is the subtractive removal inherited from below, verified on #2977.

CI

Seven of the eight required contexts are green at this head; regression has not reported yet. Zero failure conclusions among any check on the commit. Render catalog previews — the job that was being skipped two rounds ago while Preflight was red — is green.

Approving on my own verification, as a statement about f2be325b specifically. It does not clear the separate open change-request on this PR.

Review by Rames Jusso

@ukimsanov
ukimsanov force-pushed the docs/catalog-presentation branch from f2be325 to 6b76f08 Compare August 4, 2026 06:08

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at exact head 6b76f08a. The traversal hardening remains intact on the restacked tree: lexical escape, absolute paths, sibling-prefix paths, symlinked source/target/intermediate components, and inward-pointing symlinks are all refused. The real-fixture suite passes 12/12, and catalog-previews.yml still includes scripts/registry-target-paths.mjs in its paths filter.

Audited: the containment module, its full test file, the workflow trigger, and the complete inherited delta since my prior review. The removed AgentAction component and CSS have zero live references; Mint build and broken-link validation pass on the exact top-of-stack tree.

The newly restacked head has not published GitHub workflow contexts beyond WIP yet, so this approval is the code/docs verdict; branch protection must still receive its required CI contexts.

Verdict: APPROVE
Reasoning: The security boundary is filesystem-aware, covered by real fixtures, and wired to rerun when the boundary module changes; no new blocker appears in the inherited restack.

— Magi

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 6b76f08a. Approving.

The three files this PR is actually about are byte-identical to f2be325b, the head I verified last round — git diff over scripts/registry-target-paths.mjs, its test, and .github/workflows/catalog-previews.yml is empty. So the traversal analysis carries unchanged: 8 traversal vectors refused, suite 12/12, generator reproducing the committed tree exactly. Everything else at this head is inherited from #2977 and #2979.

One thing to close the loop on once CI re-triggers: Render catalog previews is the job that exercises the containment module, and it has not run on this head. The module didn't change, so I have no concern about the code — but that job is the one worth confirming green, since re-running it on module changes is the whole point of the paths-filter entry this PR adds.

CI

No workflow has run on this head. The Actions API returns zero runs for this SHA, and the only check runs present are Mintlify Deployment (skipped) and WIP. The most recent CI evidence on this branch is from the previous head.

Every branch in the stack except #2977 is in this state. The simultaneous force-push of all six looks to have left GitHub without a synchronize event for the stacked PRs — which also explains #2978's phantom conflict. Required checks hold the merge until they report, so nothing slips through; it just needs a re-trigger.

Approving on my own verification, as a statement about 6b76f08a specifically.

Review by Rames Jusso

Base automatically changed from docs/developers-reference to main August 4, 2026 09:53
Inlining the registry-target mirroring pushed prepareProjectDir to cyclomatic
24, the one complexity finding fallow attributes to this branch — the other
eight are inherited and out of scope here.

Same behaviour, expressed as a filter chain in its own named function, so
prepareProjectDir returns to what it was and the new helper stays flat.
Miguel's P1 on #2975, and it is real. `catalog-previews.yml` triggers on
`pull_request` for anything under `registry/blocks/**` or
`registry/components/**`, so `registry-item.json` arrives from the pull request
and is untrusted. `mirrorRegistryTargets` joined `files[].path` and
`files[].target` under the temp project and called `cpSync` on the result, and
`join()` walks out of its first argument. A `path` of `../../../../etc/passwd`
reads an arbitrary runner file into the project — which the job then uploads as
an artifact — and a `target` of the same shape writes an arbitrary runner path.

Both sides are now resolved and rejected when `relative(projectDir, candidate)`
is absolute or starts with `..`. Traversal that lands back inside the project
still works, so `nested/../demo.html` is unaffected.

Containment lives in `scripts/registry-target-paths.mjs` rather than inline,
because the traversal cases have to be testable and importing
`generate-catalog-previews.ts` drags in the producer. `existsSync` is injected
so the decision cannot depend on whether the target happens to exist on the
runner. Eight tests, covering traversal on each field separately, absolute
paths on each field, the sibling directory that shares the project's prefix,
and traversal that returns inside.

Verified end to end on a real tree, not only in unit tests: a manifest asking
to read `../secret.txt` and write `../pwned.txt` produces neither file, while
the legitimate entry still copies.

I introduced the wrapper when I extracted this block for a complexity finding
earlier in the stack, and did not look at what it was joining.
Miguel's second P1 on #2975, and he is right that my first fix only closed half
of it. `resolve()` and `relative()` are string operations and do not follow
links. Registry items are copied in recursively with symlinks preserved, so a
PR shipping `escape -> /tmp/outside` and declaring `target: "escape/pwned.txt"`
passed the lexical check, `mkdirSync` followed the link, and `cpSync` wrote
outside the project.

Reproduced before fixing: the old predicate returned one allowed copy and the
file appeared outside the project. Both directions were exposed — a symlinked
`path` reads a runner file in just as readily.

Containment is now filesystem-aware. No existing component of a candidate may
be a symlink, and the candidate's real location — resolved through its deepest
existing ancestor — has to sit under the project's own real path. A symlink is
refused rather than followed, even one pointing back inside the project:
nothing in the registry needs one, and following it would mean trusting the
target not to change between the check and the copy.

The tests are real fixtures now instead of string cases, because a purely
lexical suite is exactly what stayed green through the bypass. Twelve of them,
covering a symlinked target directory, a symlinked source file, a deeper path
through a symlinked component, an inward-pointing symlink, plus the lexical and
absolute cases from before.
Rames' non-blocking note on #2975. The paths filter listed the renderer but not
scripts/registry-target-paths.mjs, which it imports — so a future change to the
path-traversal defence alone would never re-run the only job that exercises it.

That is the same shape as the bug the module exists to prevent: the check is
present, the thing that would catch a regression in it is not wired to run.
@mintlify

mintlify Bot commented Aug 4, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
hyperframes 🟢 Ready View Preview Aug 4, 2026, 10:08 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 4ef1511b, now based on main after #2974 merged. Approving.

My approval at 6b76f08a lapsed to the restack, so this is a fresh statement about 4ef1511b specifically.

The rebase

187 of the 188 files are byte-identical to 6b76f08a by blob SHA. One file differs — docs/docs.json — and it is the cleanest result the check can return:

               "contributing/testing-local-changes",
+              "contributing/canary-rollouts",
               "community/adopters"

That is byte-identical to what the base itself changed over the same span (854e3fb8main, +1 line, same file, same position). The rebase absorbed the base's edit verbatim and introduced nothing of its own.

Worth saying plainly: this is the same file and the same shape as the defect on #2978 — a nav array reconciled during a restack. There it re-added a line the PR had deliberately removed. Here the result difference and the base difference are the same bytes, so there is nothing left over to explain. Invariants across the approved head, this head and main:

  • nav/redirect collisions: 0 / 0 / 0
  • dangling nav entries: 0 / 0 / 0 (307 / 308 / 308 pages against 29 redirects — the 307→308 is contributing/canary-rollouts arriving via main)
  • duplicate nav entries: 0 / 0 / 0
  • redirect sources still shadowed by a live page: 1 / 1 / 1 — only /contributing/studio-manual-dom-editing, which #2976 deletes. main was 2 last round; #2974 closed /sdk/overview on merge, as expected.

No file is deleted by this PR (2 added, 186 modified), so the resolution hazard that bit #2978 had no surface here beyond docs.json itself.

Regenerated rather than spot-checked

This is the generator PR, so the useful check is not sampling pages:

npx tsx scripts/generate-catalog-pages.ts
git status --porcelain     →  0 lines

168 pages emitted, public/catalog-index.json rewritten, and the generator reported docs.json already current with 11 groups, 168 pages. A clean git status after a full run is proof that no hand-edit is hiding anywhere in the generated tree and that the committed output is exactly what the generator produces at this head. That also independently satisfies the "no second-run drift" item in your Review focus.

Link check across the 170 changed docs files: 517 internal targets, 0 unresolved.

Containment module — carried, and re-run anyway

scripts/registry-target-paths.mjs, its test, and scripts/generate-catalog-previews.ts are byte-identical to the head I verified, so the traversal analysis carries. Re-ran it regardless since the tree moved underneath:

  • module suite: 12/12
  • traversal harness: 8/8 — ordinary file allowed; lexical ../, absolute path, symlinked directory, symlink nested in an intermediate component, inward-pointing symlink, prefix sibling, and symlink-as-read-source all refused.

The round-3 paths-filter fix survived the rebase intact, comment and all:

      # The containment module the renderer imports. Without it a change to
      # path-traversal defence alone never re-runs the job that exercises it.
      - "scripts/registry-target-paths.mjs"

And package.json wires scripts/registry-target-paths.test.mjs into test:scripts, so the new module's test actually runs in CI rather than sitting there unexecuted. Catalog Previews is green at this head — the job that filter exists to trigger.

The manifest cleanup, checked at the boundary

Twelve registry/blocks/code-snippet-apple-terminal-*/registry-item.json files drop their preview.poster. The body's framing — declared media rather than invented asset URLs — holds exactly, and the boundary is the part worth stating:

  • the 12 removed posters: 12 × 403
  • the 100 posters that survive anywhere in registry/: 100 × 200
  • the 12 videos those manifests keep: 12 × 200

So the edit removes all and only the dead posters. "The 12 really were broken" is a weaker claim than "and nothing that survives is broken", which is what makes this one safe to stop looking at.

Body vs diff

The Review focus count is stale, and it is the one line I would fix, because it tells a reviewer where not to look:

183 of the 184 changed files are deterministic output of scripts/generate-catalog-pages.ts

At this head there are 188 changed files: 170 generator output, 18 hand-written. The hand-written set is:

  • scripts/generate-catalog-pages.ts, scripts/generate-catalog-previews.ts
  • scripts/registry-target-paths.mjs and scripts/registry-target-paths.test.mjs (both new)
  • .github/workflows/catalog-previews.yml, package.json
  • the 12 registry-item.json manifests

That sentence was accurate when it was written — the containment module, its test, the workflow filter and the package.json wiring all arrived in later rounds, in response to review. It is worth updating precisely because the hand-written set now contains the security-relevant files, which is the opposite of "skip everything but the generator". All 18 are covered above or in earlier rounds.

Minor: "base is #2974" — #2974 has merged; this is now based on main.

Confirmed the rest of Verification independently: 168 pages generated, second run clean, 0 dangling navigation entries, 0 broken internal links, and no binary media or render output in the diff.

CI

8 workflow runs exist at this head, so CI genuinely ran. 47 success, 3 skipped, 3 in progress, 0 failures.

Required contexts: Semantic PR title, Build, Typecheck, Test: runtime contract and regression are green; Test, Render on windows-latest and Tests on windows-latest are still in progress. Nothing has failed.

Unlike the docs-only siblings in this stack, those two Windows contexts are actually running here rather than being skipped, because this PR touches scripts/ and package.json. That is real cross-platform coverage on the path-handling module rather than a path filter waving it through — which is the right outcome for the one PR in the stack where it matters.

Approving on the code, as a statement about 4ef1511b specifically. The three outstanding required checks remain the independent merge gate and my approval does not bypass them.

Review by Rames Jusso

@ukimsanov
ukimsanov merged commit 8364268 into main Aug 4, 2026
56 of 57 checks passed
@ukimsanov
ukimsanov deleted the docs/catalog-presentation branch August 4, 2026 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants