Skip to content

Commit 62b6da4

Browse files
authored
fix: complete security, concurrency, architecture, and test hardening (rebased on flux) (#309)
* fix(security,concurrency,arch): complete the hardening pass Security: - validatePathAllowed now fails closed without a ToolContext; tests attach a permissive test context. - Delete the dead, bypassable BoundaryChecker (no production callers). - Wrap MCP/remote tool output as untrusted external content. - Bound HTTP clients in stt/media; safewrite uses crypto/rand + O_EXCL; hooks URL validation now actually rejects loopback; plugin index reads are capped. Concurrency: - jobs: snapshot cancel status under lock; guard nil Done. - git context, spec tools, watcher: bounded exec timeouts. - planning prompt: context-aware prompt + ctx timeout. - watcher: bounded fireChange workers. - filewatcher/cron: idempotent Stop (no double-close panic). - event bus RunWaterfall: snapshot handlers under lock. - AutoCommit errors logged; AssertWritable returns an error; SessionPreparations load/wait honor a context. Architecture: - Move IsSensitivePath/ResolvePath into internal/pathsafe; drop config->tool. - Consolidate byte-unsafe truncate copies onto textutil (rune-safe). - Delete dead types.ChatClient and the dead markdown_renderer. * test(ci): de-flake tests, raise spec coverage, and tighten gates - Fix a real bug in spec extractDescription: the requirement body excludes the header, so descriptions were always empty and every ADDED/MODIFIED requirement failed SHALL/MUST validation. - Add spec tests (parse/validate/apply/DAG/config) lifting coverage from 2.4% to 24.3%, plus a fuzz target and benchmarks. - Make ContextDecay clock injectable; rewrite the timing-flaky decay tests deterministically. - Un-skip TestParallelExecution, TestIntegration_FullSessionFlow, and the two config-apply tests; remove the blanket CI -skip. - Golden test restores rootCmd globals; add make update-golden. - Add testutil.Eventually/Never. - CI: per-package coverage floors, FuzzParseDeltaSpec target, version fixture aligned to 0.0.1. * style: format with the CI-pinned gofumpt/goimports * fix(spec): don't panic on invalid UTF-8 in requirement names applyRename built a regexp with regexp.MustCompile from an unescaped requirement name; a name containing invalid UTF-8 (or a bad pattern) panicked the whole process. Use regexp.Compile and fall back to leaving the content unchanged, and ReplaceAllLiteralString so $$ in the new name is not treated as a group reference. Found by the new FuzzParseDeltaSpec target.
1 parent 49baee5 commit 62b6da4

68 files changed

Lines changed: 1014 additions & 3217 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/ci.yml‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ jobs:
135135
echo "==> validating $dir"
136136
(cd "$dir" && GOWORK=off go mod tidy -diff)
137137
(cd "$dir" && GOWORK=off go mod verify)
138-
(cd "$dir" && GOWORK=off go test ./... -count=1 -timeout=300s -skip='TestDefaultSkillDirsCrossAgent|TestCopySelectionE2E')
138+
(cd "$dir" && GOWORK=off go test ./... -count=1 -timeout=300s)
139139
done < <(find . -name go.mod -not -path './.git/*' -print | sort)
140140
141141
public-modules:
@@ -159,7 +159,7 @@ jobs:
159159
go mod download
160160
go mod verify
161161
go build -mod=readonly ./cmd/rho
162-
go test ./... -count=1 -timeout=300s -skip='TestDefaultSkillDirsCrossAgent|TestCopySelectionE2E'
162+
go test ./... -count=1 -timeout=300s
163163
164164
release-parity:
165165
name: workspace and module parity
@@ -249,7 +249,7 @@ jobs:
249249
go-version: ${{ env.GO_VERSION }}
250250
cache: true
251251
- name: Test with race detector
252-
run: go test ./... -race -count=1 -shuffle=on -coverprofile=coverage.out -covermode=atomic -timeout=300s -skip='TestDefaultSkillDirsCrossAgent|TestCopySelectionE2E'
252+
run: go test ./... -race -count=1 -shuffle=on -coverprofile=coverage.out -covermode=atomic -timeout=300s
253253
- name: Coverage summary
254254
run: |
255255
coverage=$(go tool cover -func=coverage.out | grep total | awk '{print $3}' | tr -d '%' | tail -1)
@@ -261,6 +261,23 @@ jobs:
261261
echo "::error::Coverage ${COVERAGE}% is below minimum 65%"
262262
exit 1
263263
fi
264+
- name: Per-package coverage floors
265+
run: |
266+
set -euo pipefail
267+
check() {
268+
pkg="$1"; floor="$2"
269+
cov=$(go test "$pkg" -count=1 -cover 2>/dev/null | grep -o 'coverage: [0-9.]*%' | grep -o '[0-9.]*' | tail -1)
270+
echo "$pkg coverage: ${cov:-<none>}% (floor ${floor}%)"
271+
if [ -z "$cov" ]; then echo "::error::no coverage reported for $pkg"; exit 1; fi
272+
if (( $(echo "$cov < $floor" | bc -l) )); then
273+
echo "::error::$pkg coverage ${cov}% is below floor ${floor}%"; exit 1
274+
fi
275+
}
276+
check ./internal/spec 20
277+
check ./internal/codegraph 20
278+
check ./internal/provider/gateway 20
279+
check ./internal/tool 50
280+
check ./cmd 45
264281
- name: Upload coverage
265282
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
266283
with:
@@ -540,6 +557,7 @@ jobs:
540557
go test -run='^$' -fuzz=FuzzIsSafeGitCommit -fuzztime=60s ./internal/tool
541558
go test -run='^$' -fuzz=FuzzParseMessage -fuzztime=60s ./internal/session
542559
go test -run='^$' -fuzz=FuzzParseSessionMeta -fuzztime=60s ./internal/session
560+
go test -run='^$' -fuzz=FuzzParseDeltaSpec -fuzztime=60s ./internal/spec
543561
544562
# -------------------------------------------------------------------------
545563
# 10. Smoke — build rho and verify ecosystem CLI wiring.

‎Makefile‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ api-validate: ## Validate the OpenAPI spec.
100100
bench: ## Run benchmarks.
101101
go test ./... -bench=. -benchmem -count=3 -timeout=300s
102102

103+
update-golden: ## Regenerate golden test fixtures.
104+
go test ./cmd/ -run TestGoldenHelp -update-golden -count=1
105+
103106
# ---------------------------------------------------------------------------
104107
# Quality gates.
105108
# ---------------------------------------------------------------------------

‎cmd/chat_config_save_flow_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,6 @@ func TestHandleConfigApplyCredentialsMsg_CatalogFailureDoesNotBlameProvider(t *t
225225

226226
// Skipped: integration test requiring specific flux model catalog state
227227
func TestHandleConfigApplyCredentialsMsg_ValidationFailureDoesNotBlameProvider(t *testing.T) {
228-
// TODO: enable once flux catalog fixtures pin the claude-fable-5 model state.
229-
t.Skip("requires specific flux model catalog state (claude-fable-5)")
230228
rhoconfig.InvalidateConfigUICache()
231229
store := &credentials.MapStore{}
232230
credentials.SetDefaultStore(store)
@@ -254,8 +252,6 @@ func TestHandleConfigApplyCredentialsMsg_ValidationFailureDoesNotBlameProvider(t
254252

255253
// Skipped: integration test requiring specific flux model catalog state
256254
func TestHandleConfigApplyCredentialsMsg_AuthenticationFailureBlamesKey(t *testing.T) {
257-
// TODO: enable once flux catalog fixtures pin the auth-failure model state.
258-
t.Skip("requires specific flux model catalog state")
259255
rhoconfig.InvalidateConfigUICache()
260256
store := &credentials.MapStore{}
261257
credentials.SetDefaultStore(store)

‎cmd/golden_test.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
var updateGolden = flag.Bool("update-golden", false, "update golden files")
1313

1414
func TestGoldenHelp(t *testing.T) {
15-
SetVersion("0.1.0")
15+
SetVersion("0.0.1")
1616
SetBuildDate("test")
1717

1818
tests := []struct {
@@ -25,6 +25,13 @@ func TestGoldenHelp(t *testing.T) {
2525

2626
for _, tt := range tests {
2727
t.Run(tt.name, func(t *testing.T) {
28+
// Restore the global rootCmd's output/args after this test so a
29+
// later test is not affected by our mutation.
30+
t.Cleanup(func() {
31+
rootCmd.SetOut(os.Stdout)
32+
rootCmd.SetErr(os.Stderr)
33+
rootCmd.SetArgs(nil)
34+
})
2835
buf := new(bytes.Buffer)
2936
groupRootCommands()
3037
rootCmd.SetOut(buf)

0 commit comments

Comments
 (0)