Audit I-02: Disclose LeprechaunNFT admin-mutable base URI (offchain metadata) #125

New issue

Closed

opened 2026-05-04 04:22:51 +00:00 by PlasticDigits · 4 comments

PlasticDigits commented 2026-05-04 04:22:51 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Audit reference

• Source: audits/audit_smartcontract_1777813071.md — I-02: NFT metadata remains admin-mutable
• Affected contract: LeprechaunNFT
• Finding (verbatim summary): setBaseURI allows the admin (DEFAULT_ADMIN_ROLE) to change offchain metadata URI roots. Onchain tokenTraits storage remains authoritative for stored gameplay traits, but image/metadata JSON fetched via tokenURI can change if the admin updates baseURI, so marketplace and wallet presentation can drift from what holders first saw.

Product / security stance (this issue)

Admin mutability of baseURI is intentional (e.g. hosting moves, CDN changes, corrected offchain JSON while preserving onchain trait tuples). Per the audit, the correct remediation path is not to imply immutability of offchain metadata: we must clearly disclose that administrators can update the metadata root so holders and integrators understand the trust model.

Optional future work (out of scope unless product asks for immutability): URI freeze flag, per-series provenance commitment, or immutable token URIs — only if product decides offchain presentation must be frozen.

---

Recommended fix (disclosure-first)

1. Contract documentation (NatSpec) — On LeprechaunNFT, document that:
   • baseURI / _baseURI drive tokenURI prefix for offchain JSON.
   • setBaseURI is admin-only and intentionally mutable; trait gameplay authority stays in onchain tokenTraits, but visual/metadata JSON may change when the URI root changes.
2. Participant-facing product docs — Add a short, explicit subsection under Leprechaun NFTs describing: who can change URIs, what stays fixed onchain vs what can change offchain, and why (operational flexibility). Cross-link to DEFAULT_ADMIN_ROLE / governance expectations (contracts/PARAMETERS.md).
3. Engineering / test map — Extend the Leprechaun invariant table so “Metadata admin” explicitly states holder-visible mutability and points to disclosure docs (not only “base URI can be set”).
4. Participant UX (when Collection ships) — On any surface that shows Leprechaun NFTs or links to tokenURI metadata, add concise copy that offchain metadata is served under an admin-configurable base URI (mirroring docs). Until CollectionPage leaves UnderConstruction, this can be a doc + contract NatSpec MR with a TODO anchored to CollectionPage / surfaceContent.ts collection blurb.
5. Optional transparency enhancement — Emit an event on setBaseURI (e.g. BaseURIUpdated(string newBaseURI) or indexed hash) so indexers, block explorers, and integrators can audit URI changes without diffing storage; if added, extend indexer/event docs and regression tests accordingly.

---

Regression tests (recommended)

Solidity (Foundry)

• Keep / extend contracts/test/LeprechaunNFT.t.sol: existing test_setBaseURI proves admin can change URI; add assertions that tokenURI reflects the new prefix for a minted token (if not already covered).
• If an emit is added: test that setBaseURI emits with expected parameters and that non-admin calls still revert.
• Fuzz / boundary (optional): empty string baseURI behavior documented and tested if product allows it (OpenZeppelin tokenURI concatenation — confirm valid URLs and EIP-721 expectations).

Documentation / process

• Link check: New internal links from product doc → invariants → contract path resolve in repo.
• Manual read-through: A contributor who has only read docs/product/leprechaun-nfts.md can answer: “Can the team change how my NFT looks on OpenSea without changing onchain traits?”

Frontend (when disclosure copy lands in UI)

• Vitest (optional): snapshot or string test for the disclosure snippet on CollectionPage (or shared copy module) so it cannot be deleted silently.
• Playwright: If E2E touches /collection, assert presence of disclosure test id or copy (frontend/e2e — align with docs/testing/strategy.md).

Indexer (only if BaseURIUpdated or similar is implemented)

• Decoder round-trip test for the new event; HTTP/API docs if any field is exposed.

---

Files likely to change (checklist)

Area	Path	Notes
Contract + NatSpec	contracts/src/LeprechaunNFT.sol	Document intentional mutability on baseURI, _baseURI, setBaseURI; optional event
Contract tests	contracts/test/LeprechaunNFT.t.sol	tokenURI behavior + optional emit tests
Parameters / ops	contracts/PARAMETERS.md	Leprechaun table: admin can update metadata root; link to product disclosure
Product spec	docs/product/leprechaun-nfts.md	New subsection: onchain traits vs offchain JSON; admin-mutable baseURI
Test map	docs/testing/invariants-and-business-logic.md	Expand LeprechaunNFT “Metadata admin” row; optional new invariant ID
Contracts overview	contracts/README.md	One line if table summarizes trust model
Doc map	docs/README.md	Only if new anchor headings need listing
Glossary	docs/glossary.md	Optional one-line clarification under Leprechaun NFTs
Architecture	docs/architecture/overview.md	Optional single sentence on URI mutability
Schemas	schemas/README.md, docs/schemas/README.md	Clarify schema governs JSON shape; host/path can move with baseURI
Play skill	skills/collect-leprechaun-sets/SKILL.md	Agents: prefer onchain tokenTraits for authority; warn tokenURI can change
Frontend (later)	frontend/src/pages/CollectionPage.tsx, frontend/src/lib/surfaceContent.ts	User-visible disclosure when collection is live
Indexer (optional)	indexer/src/decoder.rs, migrations, docs/indexer/design.md	Only if new event is emitted
Audit trail	audits/audit_smartcontract_1777813071.md	Do not rewrite findings; optionally add a short “Mitigation tracked in GitLab #<iid>” note in a separate maintainer edit if project convention allows

---

Acceptance criteria

1. Stated trust model: Published docs explicitly say that DEFAULT_ADMIN_ROLE may call setBaseURI, changing the prefix used for tokenURI, while tokenTraits onchain fields remain the source of truth for encoded gameplay traits.
2. No false immutability: Nowhere (docs, marketing blurbs, skill text) claim that Leprechaun metadata URIs are immutable unless product later adds a freeze mechanism.
3. Contract readability: NatSpec on LeprechaunNFT matches the above; forge doc / readers see mutability without opening the audit PDF.
4. Tests: forge test green; Leprechaun tests cover URI update behavior (and emit, if implemented).
5. Traceability: This issue ID (or MR) is referenced from the expanded invariant row or product doc for auditors and release notes.

---

Verification checklist — implementation MR

• Read LeprechaunNFT.sol and confirm NatSpec matches runtime behavior (onlyRole(DEFAULT_ADMIN_ROLE) on setBaseURI).
• Run cd contracts && FOUNDRY_PROFILE=ci forge test --match-contract LeprechaunNFT -vv (or full suite per docs/testing/strategy.md).
• Grep docs for “immutable” / “frozen” language regarding Leprechaun metadata; fix any overstatement.
• docs/product/leprechaun-nfts.md subsection is linked from docs/README.md map if required by doc style.
• Update docs/testing/invariants-and-business-logic.md Leprechaun table; ensure CI doc link check passes if the repo runs one.

Verification checklist — post-merge / release

• External communicator / launch checklist: participant-facing materials align with mutable base URI disclosure.
• If Collection page ships: visual QA that disclosure appears above the fold or in an obvious “Trust & metadata” panel.
• If event indexing added: confirm staging indexer sees URI change txs and ** docs** describe the field.

Verification checklist — auditor / reviewer quick pass

1. Open contracts/src/LeprechaunNFT.sol — setBaseURI documented as intentionally mutable admin control.
2. Open docs/product/leprechaun-nfts.md — clear onchain vs offchain authority diagram or bullet list.
3. Open test map — I-02 / metadata mutability explicitly called out as disclosed, not “resolved” by freezing (unless a separate issue adds freeze).

---

/cc Audit I-02 — remediation = disclosure (mutability intentional).

## Audit reference - **Source:** [`audits/audit_smartcontract_1777813071.md`](audits/audit_smartcontract_1777813071.md) — **I-02: NFT metadata remains admin-mutable** - **Affected contract:** `LeprechaunNFT` - **Finding (verbatim summary):** `setBaseURI` allows the admin (`DEFAULT_ADMIN_ROLE`) to change offchain metadata URI roots. Onchain `tokenTraits` storage remains authoritative for stored gameplay traits, but **image/metadata JSON fetched via `tokenURI` can change** if the admin updates `baseURI`, so marketplace and wallet presentation can **drift** from what holders first saw. ## Product / security stance (this issue) **Admin mutability of `baseURI` is intentional** (e.g. hosting moves, CDN changes, corrected offchain JSON while preserving onchain trait tuples). Per the audit, the correct remediation path is **not** to imply immutability of offchain metadata: we must **clearly disclose** that administrators can update the metadata root so holders and integrators understand the trust model. *Optional future work (out of scope unless product asks for immutability):* URI freeze flag, per-series provenance commitment, or immutable token URIs — only if product decides offchain presentation must be frozen. --- ## Recommended fix (disclosure-first) 1. **Contract documentation (NatSpec)** — On `LeprechaunNFT`, document that: - `baseURI` / `_baseURI` drive `tokenURI` prefix for offchain JSON. - **`setBaseURI` is admin-only and intentionally mutable**; trait **gameplay authority** stays in onchain `tokenTraits`, but **visual/metadata JSON** may change when the URI root changes. 2. **Participant-facing product docs** — Add a short, explicit subsection under **Leprechaun NFTs** describing: who can change URIs, what stays fixed onchain vs what can change offchain, and why (operational flexibility). Cross-link to **`DEFAULT_ADMIN_ROLE`** / governance expectations (`contracts/PARAMETERS.md`). 3. **Engineering / test map** — Extend the Leprechaun invariant table so “Metadata admin” explicitly states **holder-visible mutability** and points to disclosure docs (not only “base URI can be set”). 4. **Participant UX (when Collection ships)** — On any surface that shows Leprechaun NFTs or links to `tokenURI` metadata, add concise copy that **offchain metadata is served under an admin-configurable base URI** (mirroring docs). Until `CollectionPage` leaves `UnderConstruction`, this can be a **doc + contract NatSpec** MR with a **TODO** anchored to `CollectionPage` / `surfaceContent.ts` collection blurb. 5. **Optional transparency enhancement** — Emit an event on `setBaseURI` (e.g. `BaseURIUpdated(string newBaseURI)` or indexed hash) so indexers, block explorers, and integrators can audit URI changes without diffing storage; if added, extend indexer/event docs and regression tests accordingly. --- ## Regression tests (recommended) ### Solidity (Foundry) - **Keep / extend** [`contracts/test/LeprechaunNFT.t.sol`](contracts/test/LeprechaunNFT.t.sol): existing `test_setBaseURI` proves admin can change URI; add assertions that **`tokenURI`** reflects the new prefix for a minted token (if not already covered). - **If** an emit is added: test that **`setBaseURI` emits** with expected parameters and that non-admin calls still revert. - **Fuzz / boundary (optional):** empty string `baseURI` behavior documented and tested if product allows it (OpenZeppelin `tokenURI` concatenation — confirm valid URLs and EIP-721 expectations). ### Documentation / process - **Link check:** New internal links from product doc → invariants → contract path resolve in repo. - **Manual read-through:** A contributor who has **only** read `docs/product/leprechaun-nfts.md` can answer: “Can the team change how my NFT looks on OpenSea without changing onchain traits?” ### Frontend (when disclosure copy lands in UI) - **Vitest (optional):** snapshot or string test for the disclosure snippet on `CollectionPage` (or shared copy module) so it cannot be deleted silently. - **Playwright:** If E2E touches `/collection`, assert presence of disclosure test id or copy ([`frontend/e2e`](frontend/e2e) — align with [`docs/testing/strategy.md`](docs/testing/strategy.md)). ### Indexer (only if `BaseURIUpdated` or similar is implemented) - Decoder round-trip test for the new event; HTTP/API docs if any field is exposed. --- ## Files likely to change (checklist) | Area | Path | Notes | |------|------|--------| | Contract + NatSpec | [`contracts/src/LeprechaunNFT.sol`](contracts/src/LeprechaunNFT.sol) | Document intentional mutability on `baseURI`, `_baseURI`, `setBaseURI`; optional event | | Contract tests | [`contracts/test/LeprechaunNFT.t.sol`](contracts/test/LeprechaunNFT.t.sol) | `tokenURI` behavior + optional emit tests | | Parameters / ops | [`contracts/PARAMETERS.md`](contracts/PARAMETERS.md) | Leprechaun table: admin can update metadata root; link to product disclosure | | Product spec | [`docs/product/leprechaun-nfts.md`](docs/product/leprechaun-nfts.md) | New subsection: onchain traits vs offchain JSON; admin-mutable `baseURI` | | Test map | [`docs/testing/invariants-and-business-logic.md`](docs/testing/invariants-and-business-logic.md) | Expand **LeprechaunNFT** “Metadata admin” row; optional new invariant ID | | Contracts overview | [`contracts/README.md`](contracts/README.md) | One line if table summarizes trust model | | Doc map | [`docs/README.md`](docs/README.md) | Only if new anchor headings need listing | | Glossary | [`docs/glossary.md`](docs/glossary.md) | Optional one-line clarification under Leprechaun NFTs | | Architecture | [`docs/architecture/overview.md`](docs/architecture/overview.md) | Optional single sentence on URI mutability | | Schemas | [`schemas/README.md`](schemas/README.md), [`docs/schemas/README.md`](docs/schemas/README.md) | Clarify schema governs **JSON shape**; **host/path** can move with `baseURI` | | Play skill | [`skills/collect-leprechaun-sets/SKILL.md`](skills/collect-leprechaun-sets/SKILL.md) | Agents: prefer onchain `tokenTraits` for authority; warn `tokenURI` can change | | Frontend (later) | [`frontend/src/pages/CollectionPage.tsx`](frontend/src/pages/CollectionPage.tsx), [`frontend/src/lib/surfaceContent.ts`](frontend/src/lib/surfaceContent.ts) | User-visible disclosure when collection is live | | Indexer (optional) | [`indexer/src/decoder.rs`](indexer/src/decoder.rs), migrations, [`docs/indexer/design.md`](docs/indexer/design.md) | Only if new event is emitted | | Audit trail | [`audits/audit_smartcontract_1777813071.md`](audits/audit_smartcontract_1777813071.md) | **Do not rewrite findings**; optionally add a short “Mitigation tracked in GitLab #<iid>” note in a separate maintainer edit if project convention allows | --- ## Acceptance criteria 1. **Stated trust model:** Published docs explicitly say that **`DEFAULT_ADMIN_ROLE` may call `setBaseURI`**, changing the prefix used for **`tokenURI`**, while **`tokenTraits` onchain fields remain the source of truth for encoded gameplay traits**. 2. **No false immutability:** Nowhere (docs, marketing blurbs, skill text) claim that Leprechaun **metadata URIs** are immutable unless product later adds a freeze mechanism. 3. **Contract readability:** NatSpec on `LeprechaunNFT` matches the above; `forge doc` / readers see mutability without opening the audit PDF. 4. **Tests:** `forge test` green; Leprechaun tests cover URI update behavior (and emit, if implemented). 5. **Traceability:** This issue ID (or MR) is referenced from the expanded invariant row or product doc for auditors and release notes. --- ## Verification checklist — implementation MR - [ ] Read `LeprechaunNFT.sol` and confirm NatSpec matches runtime behavior (`onlyRole(DEFAULT_ADMIN_ROLE)` on `setBaseURI`). - [ ] Run `cd contracts && FOUNDRY_PROFILE=ci forge test --match-contract LeprechaunNFT -vv` (or full suite per [`docs/testing/strategy.md`](docs/testing/strategy.md)). - [ ] Grep docs for “immutable” / “frozen” language regarding Leprechaun metadata; fix any overstatement. - [ ] `docs/product/leprechaun-nfts.md` subsection is linked from [`docs/README.md`](docs/README.md) map if required by doc style. - [ ] Update [`docs/testing/invariants-and-business-logic.md`](docs/testing/invariants-and-business-logic.md) Leprechaun table; ensure CI doc link check passes if the repo runs one. ## Verification checklist — post-merge / release - [ ] External communicator / launch checklist: participant-facing materials align with **mutable base URI** disclosure. - [ ] If Collection page ships: visual QA that disclosure appears above the fold or in an obvious “Trust & metadata” panel. - [ ] If event indexing added: confirm staging indexer sees URI change txs and ** docs** describe the field. ## Verification checklist — auditor / reviewer quick pass 1. Open [`contracts/src/LeprechaunNFT.sol`](contracts/src/LeprechaunNFT.sol) — `setBaseURI` documented as intentionally mutable admin control. 2. Open [`docs/product/leprechaun-nfts.md`](docs/product/leprechaun-nfts.md) — clear **onchain vs offchain** authority diagram or bullet list. 3. Open test map — **I-02 / metadata mutability** explicitly called out as **disclosed**, not “resolved” by freezing (unless a separate issue adds freeze). --- /cc Audit **I-02** — remediation = **disclosure** (mutability intentional).

PlasticDigits commented 2026-05-04 08:12:53 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit 166fd6fa3a

mentioned in commit 166fd6fa3ab45ee699b8eb49c75c4bfb3c9a5bd5

PlasticDigits commented 2026-05-04 08:12:53 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit 3b9c911d4b

mentioned in commit 3b9c911d4b8983388b6097bc6dbd2c8cb666b174

PlasticDigits commented 2026-05-04 08:16:11 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Merged recovery (GitLab #125 disclosure) onto main: Leprechaun setBaseURI / mutable tokenURI documentation, BaseURIUpdated event + tests (166fd6f), merged via 3b9c911.

Contract play doc + PARAMETERS + INV-LEP-125 + Collection UI disclosure aligned; forge test / frontend Vitest green at merge.

Repo sync: working tree clean, main = origin/main, no remaining stashes or local branches to merge.

**Merged recovery (GitLab #125 disclosure)** onto `main`: Leprechaun `setBaseURI` / mutable `tokenURI` documentation, `BaseURIUpdated` event + tests (`166fd6f`), merged via `3b9c911`. Contract play doc + PARAMETERS + INV-LEP-125 + Collection UI disclosure aligned; `forge test` / frontend Vitest green at merge. Repo sync: working tree clean, `main` = `origin/main`, no remaining stashes or local branches to merge.

PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-04 08:22:13 +00:00

Brouie commented 2026-05-04 08:44:28 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in issue #138

mentioned in issue #138

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

cac-design-issue-544

renovate/configure

No results found.

Labels

Clear labels   

agent:fix_bugfix

  

agent:fix_conflicts

  

agent:fix_security

  

agent:gap_analysis

  

agent:implement

  

agent:open_issues

  

agents

  

agent:security_audit

  

agent:verify

  

area

timecurve

  

arena

  

arena-v2

  

audit

  

block:feature_review

  

block:security

  

bug

  

contracts

  

design

  

devops

  

docs

Documentation

  

documentation

  

enhancement

  

epic

Epic / umbrella issue

  

feature

  

frontend

  

indexer

  

low priority

Low priority, investigate other issues first

  

megaeth

  

operations

  

ops

Deploy and operations

  

performance

  

QA

  

qa checklist

  

referrals

  

removal

Legacy removal

  

security

  

smart-contracts

  

tech-debt

  

testing

  

time-arena

TimeArena v2 revamp

  

timecurve

  

visual-qa

No labels

agent:fix_bugfix

agent:fix_conflicts

agent:fix_security

agent:gap_analysis

agent:implement

agent:open_issues

agents

agent:security_audit

agent:verify

area

timecurve

arena

arena-v2

audit

block:feature_review

block:security

bug

contracts

design

devops

docs

documentation

enhancement

epic

feature

frontend

indexer

low priority

megaeth

operations

ops

performance

QA

qa checklist

referrals

removal

security

smart-contracts

tech-debt

testing

time-arena

timecurve

visual-qa

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

code/yieldomega#125

No description provided.