Repository navigation
Description
Summary
Byte-size settings (KindBytes) accept negative and overflowing values: settings.ParseBytes returns them with no error, Validate/Normalize pass them through, and at least one consumer spends the value as a slice capacity with no guard — so a config file, env var, or runtime override that sets http.streamBufferInit to -1 (or to a size so large it overflows int64) crashes the CLI with makeslice: cap out of range the first time it reads a streaming response.
Evidence (verified this run)
Line trace, pkgs/mcpx/internal/settings/value.go:
ParseBytes(:450-479) strips a suffix and doesstrconv.ParseFloat+int64(f * float64(mult))with no sign or range check. A negative literal parses cleanly; a huge one converts out-of-range float → int64 (on amd64/arm64:math.MinInt64).Validate(:481+, KindBytes case at :503) only checks thatParseBytesreturned no error — so every value above is a valid setting.ApplyandSetRuntimeboth gate onNormalize→Validate, so the bad value is admitted at every layer (file, env, flag, preset, runtime API).- Unguarded consumer: internal/cli/client.go:965-966 and internal/cli/opcmd.go:556-557 both do
sc.Buffer(make([]byte, 0, c.set.Bytes("http.streamBufferInit")), int(c.set.Bytes("http.streamBufferMax")))— a negative init value is a negativemakecap, which panics. - Other consumers happen to be guarded, which is why this stayed hidden: artifacts treats
MaxBytes > 0/Quota <= 0as "no ceiling" (internal/artifacts/artifacts.go:191,354-355) and logging substitutes the default whenmaxBytes <= 0(internal/logging/file.go:86-88). Those guards silently reinterpret a negative a user actually set — worth a triage look, but the crash is the headline.http.bodyLimit/http.callBodyLimitfeedhttp.MaxBytesReaderin internal/daemon/routes_secrets.go:49,114 and server.go:870 with whateverBytes()returns. - The one existing ParseBytes test (internal/settings/registry_test.go, TestByteSizesAcceptBothDecimalAndBinarySuffixes) covers only well-formed positive sizes.
Laptop probe (real internal/settings package, detached worktree at origin/main 211aa58; value.go sha256 c669f256, identical to the sandbox clone; probe + log in the goal's hidden_files/probe-2026-10-28-laneA-bytes/):
ParseBytes(-1) = -1, err=<nil>
ParseBytes(-5MB) = -5000000, err=<nil>
ParseBytes(999999999999999999999999TB) = -9223372036854775808, err=<nil>
Validate accepted byte size "-1" / "-5MB" / "999999999999999999999999TB" (KindBytes, http.streamBufferInit decl)
Normalize accepted the same three
make([]byte, 0, ParseBytes(-1)) panicked: runtime error: makeslice: cap out of range
Baseline: with the probe removed, the full existing internal/settings suite passes on the same tree.
Why it matters
The settings system is the one choke point whose whole job is keeping an invalid value out of a running process ("a runtime change that skips the parser is the one way to get an invalid value into a running process" — value.go, SetRuntime). A size is the easiest typo to make (-1 is a common "unlimited" idiom in other tools), and here it either crashes the client at first stream or — at the guarded consumers — silently turns a set limit into "no limit / default", the opposite of what was configured. Overflow needs no malice either: 999999999999999999999999TB is what a fat-fingered or generated config produces, and it lands as the most negative int64 there is.
Acceptance criteria
-
ParseBytes(orValidatefor KindBytes) refuses negative results and magnitudes that do not fit in an int64, with an error that names the offending value —ParseBytes("-1"),ParseBytes("-5MB")and the overflow case above all error. - A
KindBytessetting therefore cannot be applied at any layer (file/env/flag/preset/runtime) with such a value; the error surfaces at apply time, not at first use. - Regression tests cover negative (bare and suffixed), overflow, and the existing well-formed cases (the current suffix table test keeps passing).
- Triage confirms no consumer still spends a
Bytes()value unguarded in a way a valid value can crash (e.g. an init buffer larger than the max, at the two scanner sites) — or files that separately. - Existing internal/settings, internal/cli and internal/daemon tests keep passing.
Notes for triage
- Where the check lives (ParseBytes itself vs Validate-only) is the pin: ParseBytes is also called by
Set.Bytes()at read time and by defaults'mustBytesfor built-ins, so refusing inside ParseBytes protects every caller; refusing only in Validate leaves direct ParseBytes callers exposed. - Whether "0" keeps meaning "no ceiling/default" at the guarded consumers is existing documented behaviour — not part of this issue.
- Found by new-issue-creator lane A, 2026-10-28. Distinct from mcpx(settings): Clamp validates a dial request against the ceiling's enum, not the dial's — prompt.autonomy accepts apply/off/advise/ask #1641 (settings Clamp validating a dial against the ceiling's enum — different mechanism, same package) and mcpx(logstore): Correlations counts record cross-products, not sessions — the dashboard renders '7 sessions' for a pair that co-occurs in 2, and any Query filter fails with an ambiguous-column error #1695 (logstore Correlations).
Activity
developing-today commentedon Oct 28, 2026
CODE-680
dezren39 commentedon Oct 28, 2026
🤖 Agent note · filed
16 remaining items
dezren39 commentedon Nov 2, 2026
Agent note — triage done (task 5164, lane E)
queue/queued,task/small-fix— no hard-stop labels. Bug already CONFIRMED by reproduction in triage task 1915 (2026-10-31) and task 5537 (2026-11-01, clean main e8a360b; source unchanged atvalue.go:477). Not re-probed: two independent reproductions stand and the code path is unchanged.repo-health-code-shadow(therepo-queue.py enqueuedefault-queue trap), which pilot lanes never work, so the confirmed crash bug had no live route. This shift: cancelled the stranded shadow row (reason recorded on task 4257) and re-enqueued the fix packet (hidden_files/packets/task5164-1697-parsebytes-fix.json) intorepo-health-code-pilot. Verified end state: task 4257, queuerepo-health-code-pilot, status queued, deduperepo-health:v1:fix:developing-today/code:issue:1697.ParseBytesrefuses negative/overflowing values with an error naming the value (insideParseBytes, per the issue's triage pin), regression tests (bare/suffixed negative, overflow, existing suffix table), consumer audit for valid-value crashes at the two scanner sites, and the cross-repo port to dezren39/nix mcpx in the same pass (one issue tracks the set). Coordinate with mcpx(settings): ParseBytes accepts non-finite spellings — NaN/Inf (bare and suffixed) validate as MinInt64, a residual #1697's sign/range checks do not catch #3521 before overlapping parser edits.dezren39 commentedon Nov 2, 2026
Agent note — picked up (task 4257, lane E)
queue/4257-1697-parsebytes-fix; refuse insideParseBytes(protectsSet.Bytes/mustBytescallers); regression tests for bare/suffixed negatives, overflow, existing suffix table; consumer audit at the two scanner sites; then the dezren39/nix mcpx port in the same pass (or a SET-REMAINDER note on mcpx(settings): byte sizes accept negatives and int64 overflow — http.streamBufferInit=-1 passes validation, then panics the CLI with 'makeslice: cap out of range' at the first streaming read #1697).Merge pull request #4092 from developing-today/queue/4257-1697-parseb…
dezren39 commentedon Nov 2, 2026
Agent note — done (task 4257, lane E)
ParseBytes(pkgs/mcpx/internal/settings/value.go) refuses scaled magnitudes>= 2^63(size too large: "<value>") and negative results (size must not be negative: "<value>") — insideParseBytes, soValidate/Normalizeand the direct read-time callers are all protected.TestParseBytesRefusesNegativeAndOverflowingSizespasses (-1,-5MB,-1KiB, the TB overflow,9223372036854775808all error and name the value;0still parses; KindBytesValidaterefuses the same, accepts16MB); existing suffix table test passes;internal/cliandinternal/daemonsuites PASS;internal/settingsfailures are the identical 5 pre-existing failures verified side-by-side on clean main (document/effect/env/scriptenv declaration checks — unrelated, named by task 5537).bufio.Scanner.Buffer, whose effective max is the larger of the two — no valid value can crash there; the negative cap was the only crash vector and is now unparseable. No follow-up issue needed.