Adopt generated component manifest and audit sensitive settings - #454
Conversation
Make Pilot the first repository-native, agent-readable Weaverse theme by committing a deterministic component manifest and enforcing it in CI. Adds `.weaverse/component-manifest.json`, generated from the real registry through Vite's SSR module runner so it reflects exactly what the storefront registers. Generation never executes loaders, evaluates availability callbacks, or reads merchant content. Fixes three defects found during the audit: - `Blogs` and `BlogPost` were each registered twice (84 entries, 82 unique). - `countdown--timer` computed its schema default from `new Date()` at module load, making the manifest differ from itself on every run. Moving the fallback into the component also fixes a latent `NaN` cascade that rendered garbage when `endTime` was unset. - `aliReviewsApiKey` held a third-party bearer token as an unclassified text input; it is now `sensitive: true`. Requires `@weaverse/hydrogen@5.19.0`. The previous release pinned `@weaverse/schema@0.12.0`, which predates both `sensitive` and the `@weaverse/schema/manifest` subpath. Pilot had no PR workflow, so this adds one. Typecheck and lint were verified green against unmodified `main` first, so the gate does not import existing debt. Refs #452, Weaverse/weaverse#493
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@codex review |
CodeQL flagged the new workflow for relying on the repository default GITHUB_TOKEN permissions. The job only reads the checkout, so scope it explicitly to contents: read.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb5cb15c25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // manifest non-deterministic and its drift check unusable. An unconfigured | ||
| // timer instead falls back to one day from mount, held stable for the | ||
| // component's lifetime. | ||
| const [fallbackEndTime] = useState(() => Date.now() + ONE_DAY); |
There was a problem hiding this comment.
When the countdown preset supplies no endTime, this initializer runs independently during server rendering and browser hydration. Because calculateRemainingTime calls Date.now() again and floors every unit, crossing a millisecond boundary between those calls can make one render show 1d 0:0:0 while the other shows 0d 23:59:59; React then reports a hydration mismatch and regenerates this subtree. Use a hydration-stable initial display or establish the clock after mounting.
Useful? React with 👍 / 👎.
| const surfaces = { | ||
| presets: component.presets, | ||
| examples: component.examples, | ||
| }; |
There was a problem hiding this comment.
The recursive secret scan only visits presets and examples, so a secret-shaped defaultValue on a non-sensitive setting with an innocuous name is never checked here or by the name heuristic. Such a schema can pass this audit while committing the credential directly into the manifest; include setting defaults in the scanned surfaces as well. .agents/weaverse.mdL60-L74
Useful? React with 👍 / 👎.
Resolves package.json against main's dependency refresh: main dropped @shopify/hydrogen-react and moved to @weaverse/hydrogen ^5.19.2, which supersedes the ^5.19.0 bump this branch carried. Lockfile taken from main unchanged, since the resulting dependency set is now identical. Regenerates the manifest for main's theme version and the spacer childTypes added since this branch forked. Review fixes: - Audit now scans `settings` for secret-shaped values. `defaultValue` lives there, so a hardcoded credential under a benign name (`apiEndpoint`) passed clean before. The name heuristic misses it and the value-shape check was only looking at presets and examples. - Drops the dead REVIEWED_SAFE allowlist. Its only entry was a theme setting, which never enters the component manifest, so the branch never reached it. - Drift error no longer claims schemas changed. A version bump alone triggers it, which made the message wrong on every release PR. - Release process regenerates the manifest after the version bump, so release PRs stop failing the new gate. - Removes the biome overrides block. Main's files.includes already excludes .weaverse, so biome.json is now identical to main.
2 participants
Closes #452. Part of the agent-ready composition RFC.
Makes Pilot the first repository-native, agent-readable Weaverse theme.
What this adds
.weaverse/component-manifest.json— a committed, deterministic description of all 82 registered components: settings, non-sensitive defaults, child rules, limits, presets, loader presence, and availability metadata.DESIGN.md— brand tokens, accessibility requirements, component rules, prohibited patterns, worked examples..agents/weaverse.md— manifest regeneration, sensitivity rules, safe composition guidance.The generator loads the real registry through Vite's SSR module runner, so it sees exactly what the storefront registers. It never executes loaders, evaluates availability callbacks, or reads merchant content. Function-based rules are recorded as
{"dynamic": true}.Defects found and fixed
Duplicate registrations. 84 entries, 82 unique —
BlogsandBlogPostwere each registered twice. The generator now refuses to serialize duplicates, so this cannot silently return.Non-deterministic manifest. Two consecutive runs differed:
countdown--timercomputed its schema default fromnew Date()at module load, making drift checking structurally impossible. The fallback moved into the component.That also exposed a latent bug: an unset
endTimeproducedundefined - Date.now()→NaN, and everyNaN <= 0comparison is false, so the timer rendered garbage instead of zeros. Guarded withNumber.isFinite.Unclassified credential.
aliReviewsApiKeyheld a third-party bearer token as a plain text input. Nowsensitive: true, so the SDK redacts its value everywhere.Dependency requirement
Requires
@weaverse/hydrogen@5.19.0. The previous release pinned@weaverse/schema@0.12.0, which predates bothsensitiveand the@weaverse/schema/manifestsubpath:An npm
overridesworkaround was rejected — Pilot is the reference theme, so the stale pin would propagate downstream. Fixed properly upstream in v5.19.0.Two notes for reviewers
The heuristic is not the security boundary.
sensitive: trueis what redacts a value; name matching only catches what a human forgot to classify. The initial pattern matched bareauth, flaggingauthorName/authorTitle/authorImage/showAuthor. Allowlisting four legitimate content fields would train reviewers to ignore the check, so the pattern was tightened instead. One justified exception remains:popularSearchKeywords.Biome must not touch the manifest. Reformatting it would corrupt the canonical bytes and invalidate the hash. Using
files.includesturned out to replace the inherited list rather than merge it — warnings jumped 3 → 100. Resolved with a scopedoverridesentry that leaves the shared config intact.Verification
npm run typechecknpm run biomemainnpm run weaverse:manifest:checknpm run weaverse:auditsensitiveEvery guard was verified by making it fail first, then pass.
CI gates were checked against unmodified
mainbefore being enabled, so this workflow is green on arrival rather than importing pre-existing debt.Manifest hash:
sha256:d075f0d922f2cedcdb395cba93ee4d5a99d967015f7b9730c3d591ec81056ca5