Zero-regression hardening: close 2 RCEs + 70 correctness/robustness findings (ZRCS Steps 0-10)#12
Merged
Merged
Conversation
…ety-net
Wave 1 of the Zero-Regression Change Sequence (TDD, full -race gate):
- F05 (CRITICAL): shellQuote used strconv.Quote (double quotes) so $(...)/
backtick/$VAR reached /bin/sh -c via tmux new-session. Now POSIX
single-quote escaping. Proven inert via sh -c sentinel test.
- F06: tmux session name flowed unvalidated into the run-shell hook
(#{hook_session_name}); CreateSession now allow-lists ^uam-[a-z0-9]+-[0-9a-f]{1,16}$.
- F08: sessions.json records validated on load (drop+log) — defense-in-depth
metachar deny-list, runs in loadNoLock before migrate/re-persist.
- F09: gh pr view now passes the URL after `--`; PR URL validated against
the anchored GitHub shape.
- F50: TestStoreUpdateIsAtomicUnderConcurrency — store concurrency safety net.
- F46: secret-independent ci.yml (build/vet/gofmt/-race/lint).
Gate: build+vet+gofmt clean, 129 tests pass under -race (106 baseline).
Checkpoint commit on worktree branch only.
…variant (ZRCS F01,C1-1,F18)
…han guards (ZRCS F03,F04,F19,C1-2,F32) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ial SafeArgs (ZRCS C1-5,C2-5,F42)
…nd (ZRCS F10,F11,F12,F25,F56,F13)
…lags after agent (ZRCS F27,C2-1,F29,C2-3)
…TL (ZRCS F28,F26,F30,F58,C2-2,F53,F55)
…normalize (ZRCS F07,F33,F44)
…tempfile, PR owner/repo rehydrate (ZRCS F22,F43,F45,C2-7)
…light guard (ZRCS F20,F24,F02,F17)
…hrottle, reorder debounce (ZRCS F16,F60,C2-11,F59,F34)
…ation, wizard parity (ZRCS F14,F21,F36,C2-9,C2-8)
…ut fixes (ZRCS F40,F41,C1-3,C1-6,C2-4,F54)
…ZRCS F51,F52,F57)
…dabot/codeowners (ZRCS F48,F47,F49)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…target Go skew) golangci-lint v1.62.2 is built with go1.23 and refuses a go1.24 target; the repo already ships a version:"2" .golangci.yml. Switch to golangci-lint-action@v7 + v2.1.6 with install-mode goinstall so the linter is compiled with the repo's own toolchain. Verified locally: 0 issues.
The offline/air-gap requirement applies to the *runtime* (the production binary makes no public-internet calls — it is static and the only network call, gh pr view, is timeout-bounded), not the build environment, which has proxy access. Vendoring added ~274k lines of dependency source for no benefit here. Removed vendor/; go.sum + the module proxy provide reproducibility. Static -trimpath build, pinned CI tools, and dependabot/CODEOWNERS retained.
Resolves the CodeQL "unpinned tag for a non-immutable Action" supply-chain findings on the new workflows by pinning to immutable commit SHAs (tag kept as a trailing comment). Consistent with ZRCS F47 (pin CI tooling). - golangci/golangci-lint-action v7 - anchore/sbom-action/download-syft v0.17.9 - sigstore/cosign-installer v3.7.0 - goreleaser/goreleaser-action v6
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the full Zero-Regression Change Sequence (ZRCS) — the prioritized output of a multi-agent review of the codebase. Closes 2 confirmed RCEs and 70 additional correctness / robustness / build findings across 11 ordered steps, each landed TDD-first and gated by the full
-racesuite.Baseline was 106 tests; this PR ships 315 tests (+209), all green under
-race, with zero regressions (suite is additive-green; the original behaviors still pass).Why
A senior-architect review surfaced one CRITICAL command-execution path plus a cluster of correctness defects in the store read-modify-write path, resume/reconciliation logic, tmux protocol parsing, and the TUI. Each finding was adversarially verified (false positives discarded) and given a tests-needed-first gate so the fixes could land without breaking existing behavior.
Highlights
5a24535):shellQuoteusedstrconv.Quote(double quotes) →$(...)/backtick/$VARreached/bin/sh -cvia tmuxnew-session. Now POSIX single-quote escaping; proven inert by ash -csentinel test.run-shellsession-closed hook (#{hook_session_name}, re-expanded by tmux);CreateSessionnow allow-lists^uam-[a-z0-9]+-[0-9a-f]{1,16}$.sessions.jsonis now validated on load (drop+log), andgh pr viewpasses the URL after--.LoadSessionsread-modify-write under one lock (fixes a lost-update vsTogglePin), landed behind a new store-concurrency test; schema-downgrade guard; quarantine-safe corrupt-config load; collision-guarded insert.--continue/resume --lastinstead of re-running the original prompt in full-access mode.list-sessionsline (e.g. a cwd containing|) no longer drops the whole batch; tmux 3.4 empty-server detection; multiline-prompt handling.CGO_ENABLED=0 -trimpath), vendored deps for offline/air-gapped builds, secret-independent CI gate, pinned security tools, Dependabot/CODEOWNERS.Verification
go build ./...,go vet ./...,gofmt -l .— cleango test -race ./...— 315 passgo mod verify— all modules verifiedgovulncheck ./...— 0 reachable vulnerabilitiesGOPROXY=off go build -mod=vendor ./cmd/uam✓ → statically-linked, stripped binaryCommit map (15 atomic commits, one per step/cluster)
5a2453541d1329066715164b80ad2b33c9e297a773132ecc380099eea7343b5d63c6c328463ea144caa90eacef6b7f113acd01601Review guidance
vendor/(F48 added the vendored module tree for offline builds). The real change is 72 non-vendor files / 46 test files — review those. If vendoring is not wanted in this repo, thecd01601commit can be dropped/adjusted independently.132ecc3) and the refresh-cadence rework (28463ea).Known limitations
gosecwas not run locally (not installed in the dev env) — CIsecurity.ymlruns it (now version-pinned per F47). The RCE fixes are proven by behavioralsh -ctests.sonar.yml's secret-gated skip still reports a green check whenSONAR_TOKENis absent; the newci.ymlgate is fully secret-independent, so this is cosmetic. Flagged as a follow-up.