fix(sovereign-metadata): harden MST resolver — timeout, 4xx semantics, bounds#10
Conversation
… bounds) Post-ship hardening from a FAGAN multimodel review + self-review of the shipped MST image resolver. No behavior change for the happy path. - Bound the upstream storage-api fetch with an AbortController timeout (METADATA_FETCH_TIMEOUT_MS, default 8s). undici's global fetch has no short default — a stalled/trickling origin could otherwise tie up the request ~300s. - Stop collapsing ALL 4xx into NotFound: only 403 (unminted) / 404 (absent) → NotFoundError; 401/429/etc → thrown Error. A throttle or auth blip no longer masquerades as "token doesn't exist" (and won't surface as a 404 to API callers). - Validate the numeric tokenId at the exported fetchMstMetadata boundary too, and encodeURIComponent the path segment — the helper is now safe in isolation, not only behind getNftMetadata's validation. - Cap mapped attributes (MAX_ATTRIBUTES=64) so a hostile/buggy upstream array can't drive unbounded work/payload (real MST tokens carry ~19). Tests: +2 (abort-signal wiring, 429≠NotFound). Full suite 126 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 9e337bd56b
ℹ️ 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".
| const METADATA_FETCH_TIMEOUT_MS = Number( | ||
| process.env.METADATA_FETCH_TIMEOUT_MS ?? 8000 | ||
| ); |
There was a problem hiding this comment.
When METADATA_FETCH_TIMEOUT_MS is present but empty or non-numeric, this Number(...) expression produces 0/NaN instead of falling back to the 8s default; setTimeout(..., 0/NaN) then aborts real MST metadata fetches almost immediately in that deployment configuration. Since this is an ops override, validate that the parsed value is a finite positive duration before using it, and otherwise keep the default.
Useful? React with 👍 / 👎.
1 participant
Post-ship hardening of the MST image resolver (shipped in #9), from a FAGAN multimodel review (Opus-skeptic + GPT-5.5 + Composer) cross-checked with self-review. Happy path unchanged; 126 tests pass.
fetchwith anAbortController(METADATA_FETCH_TIMEOUT_MS, default 8s). undici's globalfetchhas no short default; a stalled/trickling origin could tie up the request ~300s. (FAGAN-confirmed + self-flagged.)403/404→NotFoundError;401/429/etc → thrownError. A throttle or auth blip no longer masquerades as "token absent" (or surfaces as a misleading 404 to API callers).tokenIdcheck +encodeURIComponentinfetchMstMetadata/mstMetadataUrl, so they're safe in isolation, not only behindgetNftMetadata.MAX_ATTRIBUTES=64) so a hostile/buggy upstream array can't drive unbounded work/payload.Tests: +2 (abort-signal wiring ·
429 ≠ NotFound).context: part of the MST mint-image enrichment closeout. Companion PR in freeside-characters hardens the consumer (HTTP client self-catch + image-URL allowlist).
🤖 Generated with Claude Code