Repository navigation
Description
Hardening items from the adversarial review of slice 6a (#354). None changes the contract or the conformance result.
- Pin reads the temp manifest by path but renames it by handle.
scaffold/pin.goloadsfilepath.Join(dirAbs, tmp)withplugin.LoadFile, then renames through theos.Roothandle. If the directory is swapped between the two, the printed digest could describe a different file. Read the temp file back through the root handle, hash and decode those bytes, and print that digest. - The skeleton trims leading whitespace before
json.Valid.{...}(C2 A0) and U+0085 are served by the skeleton but refused with -32700 bylab/plugins/internal/mcpserver. Match the kit: validate the raw line. - Printed build commands ignore
go.workandGOFLAGS. Theplugin newnext steps and the README should setGOWORK=off(and say why), as the tests do. - Error reasons for authors. The skeleton always emits
reason: "failed". Add a helper and a README section forauth-missing,rate-limitedandnetwork, plus a per-tool output bound and handler timeout like the mcpserver kit's. - Adopting an existing empty
--dirshould refuse a group- or world-writable directory, whichplugin testwould later reject throughCheckPlacement.
Done when
go test -timeout 20m ./internal/plugin/scaffold/... ./internal/cli/...passes fromlab/.
Scope (files): lab/internal/plugin/scaffold/pin.go, lab/internal/plugin/scaffold/scaffold.go, the Go skeleton templates under lab/internal/plugin/scaffold/templates/go/ (main.go.tmpl, README.md.tmpl), lab/internal/cli/plugin_new.go, lab/plugins/README.md (the GOWORK note and the error-reason section; keep the <!-- forgecell:example-tracker --> block byte-identical), and a new lab/internal/plugin/scaffold/hardening_test.go. The Python and TypeScript skeletons are out of scope unless item 2 or 4 applies to them identically, in which case name the template files in the plan.
Plan checks for this slice
Use one combined targeted go test -timeout 20m ./internal/plugin/... ./internal/cli/... check. Keep every existing _test.go unchanged unless the plan names a base test that asserts behaviour this slice replaces by design. Every plan check leaves reviewedAcceptance and independentProvenance empty.
Review checklist (read before coding)
The adversarial review checks these every time. Recent slices failed review on all of them, so build them in from the start:
- Bounds: sizes, counts, time and total work are bounded, and hitting a bound fails safe (incomplete or truncated, never silently accepted).
- Filesystem safety: paths cannot traverse. Use
LstatandO_NOFOLLOW, and check symlinks and ownership. Watch for TOCTOU races on shared dirs (/tmp). Only ever delete Forgecell-owned, marked paths. - Untrusted input: issue text, repository files, model and tool output, and ledgers can be malformed, huge or hostile. Validate them, never interpolate them into HTML or a shell, and never let them trigger retries or approvals.
- No local leaks: no absolute local paths, environment variables or credentials in output, logs, the UI or ledgers.
- Contract changes: a changed adapter invocation bumps the protocol, and approved digests still bind exactly what was shown.
- Tests: deterministic. Use no wall-clock budgets (use readiness markers or seams), write nothing into the user's real temp or home, and cover the failure path as well as the happy path.
Plan checks (for intake)
Check category must be one of regression, candidate or setup. Use targeted package tests only as plain go test -timeout 20m commands (for example go test -timeout 20m ./internal/<pkg>/...; without -timeout, go test stops a package after 10 minutes, and cli and molecule exceed that on a loaded machine); the protected view adds -json itself when protected deltas are declared. The Formula baseline already runs the full suite once, so don't add another full go test ./... check, and don't repeat its go vet or gofmt checks. A plan may hold at most 16 checks in total, counting the 5 Formula baseline checks, so prefer one combined targeted command over one check per package.
- Keep base tests compiling: the protected view runs the base
_test.gofiles against your code. Never change the signature of an existing function, method or type that a base test file calls. Add a new function or field instead. A protected build failure can't be declared as an expected delta. - Keep base tests runnable: changed defaults or behaviour must not make a base test hang or panic. A package that aborts in the protected view hides every test after it, and nothing can be declared. Keep the old default when unconfigured and opt in to the new one.
- Plan checks: the operator's visual or browser check is a manual step after delivery, never a plan check. Every check leaves
reviewedAcceptanceandindependentProvenanceempty. - Plan structure: new files need a parent component (in this repo, under
lab/; a new root-level file blocks intake). A check'sdefinitionsmust never include a file the change edits. A file the check only reads as data under test is not a definition. - Scope lists files, not directories: list every new file path, including each golden fixture under
testdata/. A directory entry doesn't cover the files inside it, and the scope audit blocks them.
Activity
plugin new/pin hardening: handle-based pin digest, raw-line JSON, GOW…
DanielNetzer commentedon Nov 4, 2026
Fixed in #374.