Approved workflow

README.md states that docs/dependencies.json gives 'SHA-256 hashes [that] cover every vendored file'. Recomputing SHA-256 over the tree shows the forge-std v1.9.4 entry is wrong for 7 of 29 files: src/StdAssertions.sol, src/StdJson.sol, src/StdToml.sol, src/Vm.sol, src/console.sol, src/interfaces/IMulticall3.sol and src/mocks/MockERC721.sol.

Every OpenZeppelin v5.1.0 hash (the only vendored code that reaches deployed bytecode) matches, so this does not affect LaunchToken or MossExperimentRegistry runtime. It does mean the supply-chain evidence the README points reviewers to cannot be used to attest the test toolchain: either the listed hashes were taken from a different forge-std revision or the files were altered after the manifest was written.

CRLF/trailing-newline normalisation does not explain the mismatch (tested). Upstream comparison could not be run (network denied), so which side is wrong is unverified; the defect is that the project's own provenance document and tree disagree.

Not a permission bypass; recorded as the privileged-role trust assumption the Access Control pass requires to be documented. (1) owner is immutable and the ABI exposes no transfer, renounce, accept, initialize or grantRole path, so a lost or compromised $owner key permanently freezes approve/reject/retire (every call from any other address reverts Unauthorized) and the only remedy is deploying a new registry. The README already discloses this.

(2) The owner_ == msg.sender check is a convenience guard against the documented 'owner = factory' mistake; it only holds when the factory is the direct CREATE caller. If the launch route ever deploys through an intermediate helper contract, $owner == factory passes the constructor and yields an owner that can never act.

The real control is the independent review of the concrete $owner value in launch.json, which does not exist yet in this tree; that review must confirm the resolved owner is the intended policy multisig and not the factory, token, zero address or registry.

Proposal ids are assigned by ++proposalCount at propose() time and propose() is permissionless. approve() takes only the id, expectedActiveId and decisionHash; it does not take (or re-check) the implementation address, codeHash or contentHash the owner actually reviewed. Nothing on chain binds the owner's endorsement to the reviewed content except the id.

If the owner (typically a multisig whose signers collect signatures ahead of time) signs approve(N, ...) for an announced proposal before that proposal is mined, any unprivileged actor can front-run the researcher's propose() so that id N is theirs; the owner's transaction then marks the attacker's implementation Active for that key and the frontend shows it as officially endorsed.

Precondition: the owner approves an id it has not yet observed on chain. When the owner only ever approves ids that already exist (the README's documented procedure), the id-to-content binding is immutable and this cannot happen, so the impact is limited to an operational race.

Economic impact: no on-chain funds move, but a falsely endorsed experiment is exactly the social-engineering surface the catalog exists to prevent. Minimal fix that preserves the design: add the reviewed implementation and contentHash (or a keccak of the Proposal payload) as approve() parameters and revert on mismatch, so an approval is cryptographically bound to what was reviewed rather than to a race-assignable counter.

README.md line 31 states that docs/dependencies.json's SHA-256 hashes 'cover every vendored file'. Recomputing sha256 over the tree shows 7 of the 28 forge-std files differ from their recorded hash: src/StdAssertions.sol, src/StdJson.sol, src/StdToml.sol, src/Vm.sol, src/console.sol, src/interfaces/IMulticall3.sol, src/mocks/MockERC721.sol.

The recorded hashes equal the upstream forge-std v1.9.4 archive contents (archive sha256 9bf19180... matches the recorded archiveSha256), and a whitespace-insensitive comparison shows the vendored copies differ from upstream only by formatter reflow (forge fmt line wrapping), so there is no semantic tampering and no production bytecode is affected (forge-std is test-only). All 6 OpenZeppelin files match upstream v5.1.0 byte for byte.

The defect is that the provenance manifest the reviewer and verifier are told to rely on is stale for the tree that is committed: an integrity check that anyone runs on this commit reports mismatches, which is indistinguishable at first glance from a modified dependency.

Fix: either re-vendor the 7 files byte-identical to upstream (and exclude lib/ from forge fmt), or regenerate the recorded per-file hashes from the committed tree and note that formatting was applied.

Documented design, reported as a trust assumption rather than a defect. The policy-resolved $owner is the only address that can approve, reject or retire, and the address can never change.

Two consequences the launch reviewer should weigh against the policy owner that the manifest will supply: (1) if the owner key or multisig quorum is lost, every pending proposal stays Pending and every active endorsement stays Active forever, with no path to replace or retire them; (2) a compromised owner can endorse arbitrary external contracts immediately, with no delay in which holders could react.

The registry itself holds no funds and makes no calls, so neither case can move token supply; the harm is confined to the credibility of the endorsement signal the frontend displays. The constructor correctly refuses the factory (msg.sender), the token, the registry and the zero address as owner, so the remaining risk is entirely in which address policy resolves $owner to.

Boundary: the guard assumes that an address with non-empty code is a deployed contract whose direct runtime bytes can be reviewed and hashed. On a chain with EIP-7702 enabled (Prague-level networks; Arbitrum Orbit chains gain it with ArbOS 40 or later, and Robinhood Chain is an Orbit chain, so this depends on its ArbOS level), an externally owned account whose key signed a delegation carries 23 bytes of code (0xef0100 followed by the delegate address).

EXTCODESIZE returns 23 and EXTCODEHASH returns keccak256 of that designator, so the check at line 82 passes and the stored codeHash commits only to the delegate pointer.

The key holder can re-delegate at any time with no contract deployment; the registry then reports isActive(id) == false while activeProposal(key) still names the entry until the owner retires it. docs/ABI.md's error table says non-contract candidates are rejected with InvalidImplementation, which is not true for such accounts.

Impact is limited: the registry never calls the implementation and the owner reviews before endorsing, so this is a guard/documentation gap and an item for the owner's review checklist, not a loss path. Minimal fix that keeps the intended behaviour: in propose(), additionally reject implementations whose code starts with the 3-byte prefix 0xef0100 (equivalently code.length == 23 with that prefix), and state in README/ABI docs that delegated EOAs are treated as non-contracts.

Alternatively keep the behaviour and correct the ABI doc wording.

Boundary of the test harness, not the contract: when vm.expectRevert precedes a direct new in the test body and the constructor reverts, Solidity bubbles the revert out of the test function itself. Forge marks the test PASS because the expectation was satisfied, but every statement after that first new is dead. In test_constructorRejectsMissingTokenCode (lines 55-60) the second case, a token address with no code (RESEARCHER), never runs.

In test_constructorRejectsInvalidOwnerIncludingFactory (lines 62-69) only owner == address(0) runs; owner == address(this) (the msg.sender/factory branch) and owner == address(token) never run. The traces confirm it: both tests end at the first new with reported gas of 4107 and 6302, far below one constructor execution. The README claims these branches are covered.

The constructor code itself is correct for every branch (verified separately through an external deployer contract: no-code token, zero/self/token/msg.sender owner all revert with the expected error and a valid pair deploys), so this is a coverage defect only.

Minimal fix: deploy through a helper contract's external function (or vm.deployCode with try/catch) so each expectRevert applies to an external call, or split each case into its own test.

Merged from audit_permissions and audit_economics (same root cause). README.md line 31 says the SHA-256 hashes in docs/dependencies.json 'cover every vendored file', and docs/VALIDATION.md line 11 states 'All 34 dependency files match docs/dependencies.json'.

Recomputing SHA-256 over the committed tree shows 7 of the 28 forge-std entries do not match: src/StdAssertions.sol, src/StdJson.sol, src/StdToml.sol, src/Vm.sol, src/console.sol, src/interfaces/IMulticall3.sol, src/mocks/MockERC721.sol. All 6 OpenZeppelin v5.1.0 files (the only vendored code that reaches deployed bytecode) and the other 21 forge-std files match.

I fetched the upstream forge-std v1.9.4 archive (sha256 9bf19180...96a15, equal to the recorded archiveSha256): the recorded per-file hashes are the upstream values, and each of the 7 vendored files is identical to upstream after stripping whitespace, so the committed copies were reformatted (forge fmt --check passes on them, which upstream does not guarantee). No semantic tampering, no production bytecode affected; LaunchToken and MossExperimentRegistry are unaffected.

The defect is that the provenance document the README and VALIDATION.md direct reviewers and the offline verifier to rely on fails its own check on this commit, and the VALIDATION.md claim is false for the tree it describes (it also reports 42 passing tests where the committed suite now has 56).

Fix: either re-vendor the 7 files byte-identical to upstream, or regenerate the per-file hashes from the committed tree and note the formatting in VALIDATION.md.

From audit_math; reproduced. When vm.expectRevert precedes a direct new MossExperimentRegistry(...) in the test body, the constructor revert propagates out of the test function itself (trace ends with the test frame reverting InvalidToken()/InvalidOwner()). Forge marks the test PASS because the expectation was met, but every statement after the first new is dead.

In test_constructorRejectsMissingTokenCode (lines 55-60) the RESEARCHER no-code-token case never runs; in test_constructorRejectsInvalidOwnerIncludingFactory (lines 62-69) only owner==address(0) runs, and the owner==address(this) and owner==address(token) cases never run. Reported gas (4107 and 6302) is below a single constructor execution.

The owner==msg.sender branch is separately covered by ProjectDeployment.t.sol test_factoryCannotAccidentallyBeConfiguredAsOwner through an external call, but the no-code-token and owner==token branches are not exercised anywhere in the suite, contrary to README line 55 and VALIDATION.md.

The contract is correct for every branch (verified via an external deployer contract: no-code token, zero/token/deployer owner each revert with the expected error, a valid pair deploys), so this is a test-coverage defect only. Note the contrast with RegistryAdversarial.t.sol:136-137, where the same pattern around new PrematureExperiment(registry) does continue, because that revert is caught at the CREATE frame; the direct registry new is not.

Fix: deploy through a helper contract's external function so expectRevert applies to an external call, or split each case into its own test.

From audit_economics; reproduced. Ids are assigned by ++proposalCount in the permissionless propose() (line 86). approve() takes only the id, the expected active id and the decision hash; it never re-checks the implementation address, codeHash or contentHash the owner reviewed, so nothing on chain ties the endorsement to reviewed content except the race-assignable counter.

If the owner (in practice a multisig collecting signatures ahead of time) signs approve(N, ...) for an announced proposal before that proposal is mined, any account can front-run propose() so that id N holds a different implementation; the owner's transaction then marks that implementation Active under the owner's decision document and isActive(N) returns true.

Precondition: the owner approves an id it has not yet observed on chain. Following the README procedure (observe the id, review, then approve) prevents it, since proposal contents are immutable once stored, so impact is an operational race on the endorsement signal rather than a loss path: the registry holds no funds and never calls the implementation.

Minimal fix that preserves the design: add the reviewed implementation and contentHash (or keccak256 of the Proposal payload) as approve() parameters and revert on mismatch, so an approval is bound to what was reviewed.

From audit_math; reproduced. On a chain with EIP-7702 active, an externally owned account that signed a delegation carries 23 bytes of code (0xef0100 || delegate address). EXTCODESIZE returns 23 and EXTCODEHASH returns keccak256 of the designator, so the guard at line 82 passes and the stored codeHash (line 87) commits only to the delegate pointer, not to any runtime the owner can review.

The key holder can re-delegate at any time without a contract deployment; isActive(id) then returns false while activeProposal(key) still names the entry until the owner retires it. docs/ABI.md line 49 states that non-contract candidates revert with InvalidImplementation, which is not true for such accounts. The project compiles for evm_version paris and the deployment chain's 7702 status is unverified, so this is conditional on the chain.

Impact is limited: the registry never calls the implementation and the owner reviews before endorsing, so this is a guard/documentation gap, not a loss path. Minimal fix that keeps intended behaviour: in propose() additionally reject implementations whose code is exactly 23 bytes beginning with 0xef0100 and say so in README/ABI docs; or keep the behaviour and correct the ABI.md wording.

Merged from audit_permissions and audit_economics (same privileged-role trust assumption; not a permission bypass). (1) owner is immutable and the ABI exposes no transfer, renounce, accept, initialize or grantRole path, so a lost or compromised $owner key permanently freezes approve/reject/retire (every pending proposal stays Pending, every active endorsement stays Active), and a compromised owner can endorse arbitrary external contracts immediately with no delay.

The registry holds no funds and makes no calls, so neither case can move token supply; the harm is confined to the credibility of the endorsement signal the frontend displays. README line 53 and docs/REVIEW-NOTES.md already disclose this.

(2) The constructor's owner_ == msg.sender check at line 63 only catches the factory when the factory is the direct CREATE caller; if the launch route ever deployed through an intermediate helper, $owner == factory would pass and yield an owner that can never act. launch.json resolves owner from $owner, so the controlling check is the independent confirmation that the policy's resolved $owner is the intended, recoverable multisig and not the factory, token, zero address or registry.

Recorded so the manifest/policy reviewer weighs the concrete $owner value; no code change is required under the accepted design.

From audit_economics; confirmed against the tree. The brief in .imd/reads/workflow.md asks for 'same concept' as the linked post (a token whose trading mechanics evolve via owner-approved on-chain changes). The launch rules fix the pool behind a factory-supplied initialization-only guard, forbid proxies/delegatecall, and require a plain fixed-supply token, so no contributor contract can change swap rules.

The delivered LaunchToken is the standard token and MossExperimentRegistry is a public endorsement catalog with no pool, hook or fee state; registry approval changes nothing about trading. README line 7, docs/REVIEW-NOTES.md and the launch.json notes all record this difference.

Reported so the gap stays visible to the requester, who must accept the reduced scope or commission a different architecture outside this launch route; it is not a defect in the code, which matches its own README and the launch constraints.