Hold .soldeerignore against the repo root, and stop shipping .audit/ - #133
Hold .soldeerignore against the repo root, and stop shipping .audit/#133thedavidmeister wants to merge 4 commits into
Conversation
…cumentation `.vscode` and `/docs` matched nothing and never have; `.audit/` was at the root and unexcluded, so its run stamps shipped in the package. Closes #91 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the suite leaves an empty `meta/` at the root, which nothing excluded, so it would ship in any package built after a test run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 1 minute Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Delete `test/package/SoldeerIgnore.t.sol` and revert the repo-root
`{ access = "read", path = "./" }` fs_permissions grant, returning
`fs_permissions` to its base three entries.
The `.soldeerignore` and `.gitignore` changes stay: `.audit/` stops shipping and
`meta/` can no longer leak.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #91
No test guards this change, and it can regress silently
This PR ships no forge test. The check it used to carry —
test/package/SoldeerIgnore.t.sol, which listed the repo root and held.soldeerignoreagainst it in both directions — has been deleted, and therepo-root
{ access = "read", path = "./" }fs_permissionsgrant it needed hasbeen reverted, so
fs_permissionsis back to its base three entries.That means: nothing in this repo will notice if this regresses. A new
top-level path can start shipping in the soldeer package because nobody added it
to
.soldeerignore, and a.soldeerignoreentry can go dead — matching nothing,exactly the
.vscodeand/docsdefect this PR fixes — and the suite will staygreen through either. There is no test to strengthen and no assertion to read;
the only thing standing between the package and the next leak is someone editing
.soldeerignoreby hand, correctly, at the same time they add a root path.The check is being rehomed to rainlanguage/rainix#317 — "Six repos' worth of
static analysis is being hand-rolled in Solidity because
rainix-statichas nohome for repo-wide lint checks". It is a repo-root packaging rule, not a
statement about any Solidity in
src/, and it belongs in shared static analysisthat every rain repo gets, not in one repo's
forge testreading its owndirectory listing through a cheatcode. Until that lands, this is unguarded.
Why the test was here at all
The fixer brief that produced this PR said every fix ships a test and
verify-by-hand is never acceptable, with no exemption for changes that have no
behaviour. That was wrong and has been corrected.
.soldeerignoreand.gitignoreare configuration files with no Solidity behaviour, and the testwritten to satisfy that brief was hand-rolled static analysis in Solidity: 174
lines whose subject was a directory listing and two text files, with two of its
four tests asserting only against its own hardcoded
shipped()set — itself ahand-maintained second definition of the package, which is the same class of
defect as the
.soldeerignoredrift it was written to catch.What ships here
.soldeerignoreand.gitignore. This is the change with the real consequence:.audit/stops shipping, andmeta/can no longer leak.Dead entries, the finding as filed.
.vscodeand/docsmatched nothing andnever have —
git log --all -- .vscodeandgit log --all -- docsare bothempty. Removed.
.audit/was shipping. The audit stamp commit added.audit/runs.jsonland.audit/scope.jsonat the root..soldeerignoreexcluded/auditbut not.audit, so both files are inside the published package. Confirmed against theregistry rather than inferred —
unzip -Z1on the0.1.12and0.1.16zipspulled from
api.soldeer.xyzlists:Now excluded via
/.audit.meta/was one test run away from shipping.LibCodeGenDescribedByMetaHashConstantStringTest.setUpdoesvm.createDir("meta"),and the directory is left behind at the repo root. It was in neither
.gitignorenor
.soldeerignore, so any package built from a tree that has run the suitecarries it. It is absent from
0.1.12and0.1.16because the publish job doesnot run the suite, so this is latent rather than already leaked — but it is
latent by luck, not by design. Now excluded, and gitignored alongside
cache,dependenciesandout, which is what the rest of the suite's output alreadygets.
The issue's note about keeping
.DS_Storeis right and the entry is untouched:nothing here produces it and git is not told about it, but macOS writes it into
any directory it browses.
Where the issue's proposed fix was incomplete
The issue said "delete lines 8 and 12" and it was right about those two lines. It
did not name
.auditormeta, and neither would have been found by reading thelist —
.auditneeded the published zip andmetaneeded the suite to run. Bothare the same defect the issue describes, so both are fixed here.
Interaction with #121
#121 (issue #84) removes
.gas-snapshotand its.soldeerignoreentry. The twochanges do not conflict textually. With no test in this repo holding the list
against the root, nothing forces whichever lands second to do both halves —
dropping the file without the entry, or the entry without the file, is now silent
in both directions. That is a concrete instance of the regression this PR is
unguarded against.
QA
nix develop -c forge test—Ran 18 test suites: 142 tests passed, 0 failed, 0 skipped (142 total tests). Before the strip, on the samemerge commit,
Ran 19 test suites: 146 tests passed, 0 failed, 0 skipped (146 total tests)— the count drops by exactly the fourtest/package/SoldeerIgnore.t.soltests removed, and nothing else changes.nix develop -c forge fmt --check— clean, no diff.origin/mainis merged in (merge, not rebase), which brings Declare every tooling builderviewso an implementation can read state #124.foundry.tomlis now byte-identical toorigin/main—git diff origin/main -- foundry.tomlis empty.unzip -Z1over the0.1.12and0.1.16zips fetched fromapi.soldeer.xyzgives the actual shipped file list, and
.audit/runs.jsonland.audit/scope.jsonare in it, which is the leak. For the dead entries theoracle is
git log --all -- .vscodeandgit log --all -- docs, both empty,so neither path has ever existed here. For
metathe oracle is the suite's ownvm.createDir("meta")plus the directory left at the root after a run.