fix: derive skill publish metadata from storage#2470
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Codex review: needs maintainer review before merge. Reviewed June 2, 2026, 3:44 AM ET / 07:44 UTC. Summary Reproducibility: yes. from source inspection, not runtime execution: current main validates size and content type from args.files before reading storage, so mismatched caller metadata can affect enforcement. The PR adds focused tests for that path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land a central publish-layer fix that treats stored bytes as the source of truth while preserving the existing publish payload shape and focused regression coverage. Do we have a high-confidence way to reproduce the issue? Yes from source inspection, not runtime execution: current main validates size and content type from args.files before reading storage, so mismatched caller metadata can affect enforcement. The PR adds focused tests for that path. Is this the best way to solve the issue? Yes, deriving metadata inside publishVersionForUser is the narrow maintainable fix because UI, CLI, API, imports, and restores all converge there. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against dcbc38999f1a. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
Pull request overview
Replaces caller-supplied skill publish metadata with values derived from stored blobs, ensuring size, contentType, and sha256 reflect the actual storage contents and that file/total size limits cannot be bypassed by lying inputs.
Changes:
- Extract a new
derivePublishFilesFromStoragehelper that loads each blob, validates content-type/size/total-size, and recomputes sha256. - Reorder
publishVersionForUserto apply the storage-derived metadata before downstream consumers (readme detection, quality checks, etc.). - Add unit tests that cover happy-path metadata derivation and oversized-stored-blob rejection when caller metadata understates the size.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| convex/lib/skillPublish.ts | Adds storage-driven metadata derivation, sha256 hashing, and rewires publish flow to use it. |
| convex/lib/skillPublish.test.ts | Adds regression tests for derivePublishFilesFromStorage, including oversized-blob rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
publishVersionForUser.Tests