[fix] preserve the step budget and script-cache choice across Reset - #29
Conversation
Reset replaces the machine with a fresh one that defaults to no step budget,
but SetMaxExecutionSteps only ever pushed the budget onto the current machine
with no field on the Box, so New -> SetMaxExecutionSteps -> Run -> Reset -> Run
left every run from the second onward with NO CPU-DoS guard — on exactly the
serial-reuse-of-one-Box path Reset is meant for. That contradicts Reset's own
doc ("keeping the Box's ... limits"), and it is asymmetric with maxOutputEntries,
which is a Box field read at run time and therefore already survived Reset.
The budget is now a Box field (maxSteps), and prepareEnv re-applies it to each
machine on the first run after construction/Reset, the same shape as the other
Box-level configuration. SetScriptCache had the same machine-only flaw (Reset's
newStarMachine re-enables the cache), so its choice is stored on the Box and
replayed too.
Test: TestMaxExecutionStepsSurvivesReset — set a budget, hit it, Reset, and the
next run still hits MaxStepsExceededError.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
🟢 Coverage 100.00% diff coverage · +0.01% coverage variation
Metric Results Coverage variation ✅ +0.01% coverage variation (-1.00%) Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (fd18a0f) Report Missing Report Missing Report Missing Head commit (fb4d38b) 1169 (+15) 1160 (+15) 99.23% (+0.01%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#29) 17 17 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
==========================================
+ Coverage 98.95% 98.96% +0.01%
==========================================
Files 12 12
Lines 862 871 +9
==========================================
+ Hits 853 862 +9
Misses 5 5
Partials 4 4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem
SetMaxExecutionStepspushed the CPU-step budget onto the currentstarlet.Machineonly — theStarboxkept no copy.Reset()swaps in a fresh machine (newStarMachine), which defaults to no step limit and an enabled script cache. So after the first run the guard silently vanished:This directly contradicts
Reset's own doc ("keeping the Box's limits"), andResetis exactly the sanctioned path for serially reusing one Box across untrusted scripts — so a security-minded host hits the dangerous case.SetScriptCache(nil)(disable) had the symmetric bug:Resetre-enabled the cache.maxOutputEntriesdid not have this bug because it is a struct field re-read at run time — the asymmetry confirmed the root cause.Fix
maxSteps/scriptCache/scriptCacheSettoStarboxfields; the setters store them.Reset()re-applies both to the fresh machine eagerly (right afternewStarMachine), not lazily at the next run — so a caller that reaches for the raw machine viaGetMachine()immediately afterReset()also sees the guard, closing that bypass.applyScriptCache()respectsscriptCacheSet: an untouched cache stays default-enabled; an explicitSetScriptCache(nil)stays disabled across everyReset.Test
TestMaxExecutionStepsSurvivesReset(test-first): set budget → Run (hitsMaxStepsExceededError) →Reset()→ Run → still hitsMaxStepsExceededError.Verification
gofmt/go vetclean;go test,go test -race -count=2, anddocker golang:1.19 go test -raceall green.