diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 76f2f11..ab2b5f4 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Import Discovery and Scheduling Review Findings -Status: in progress; issues 1–4 complete, two findings remain. +Status: in progress; issues 1–5 complete, one finding remains. Reviewed implementation: `aa93aed..15943e2`, ending at `15943e233ca3cd1286c03c0e7085871db344cd4b`, against the accepted @@ -150,7 +150,7 @@ loops, plus public host/native directory/glob loading coverage. Finite cases failed before the change; all pass now alongside existing traversal-cycle and containment tests. The full everyday check passed; no new limit was added. -## 5. [ ] P2 — Validate the complete depth token before saturating its value +## 5. [x] P2 — Validate the complete depth token before saturating its value Owner: [parseImportDepth](../../internal/engine/import_spec.go), around lines 423–430. @@ -171,6 +171,12 @@ large valid nonnegative integers and numeric `-0`; do not introduce a machine-si maximum. Cover invalid suffixes after the overflow point as well as ordinary invalid depths and valid large integers. +Completed: depth parsing saturates its accumulator without stopping token +validation. Public absent-directory regressions now reject fractional/exponent +suffixes after overflow and retain valid huge integers and `-0`. The two invalid +cases failed before the fix. Existing depth/specification coverage and the full +everyday check passed; the accepted integer range was not narrowed. + ## 6. [ ] P2 — Apply the active import-depth limit to actual inclusion edges Owner: [walkDeclarations](../../internal/engine/imports.go), around lines 403–405. diff --git a/facade_import_defaults_test.go b/facade_import_defaults_test.go index 6125f1a..7cf6680 100644 --- a/facade_import_defaults_test.go +++ b/facade_import_defaults_test.go @@ -10,6 +10,36 @@ import ( "gitea.maximumdirect.net/eric/ConfigKit/internal/engine" ) +func TestImportDepthChecksSuffixAfterSaturation(t *testing.T) { + for _, test := range []struct { + depth string + valid bool + }{ + {"999999999999999999999999.5", false}, + {"999999999999999999999999e1", false}, + {"999999999999999999999999", true}, + {"-0", true}, + } { + t.Run(test.depth, func(t *testing.T) { + main := writeFacadeImportFile(t, t.TempDir(), "main.yml", "value: true\nconfigkit:\n imports:\n - dir: absent\n recursive: true\n depth: "+test.depth+"\n") + result, err := Load[map[string]any](File(main), Options[map[string]any]{Imports: true}) + if test.valid { + if err != nil || result.Config["value"] != true { + t.Fatalf("valid depth: %v", err) + } + return + } + var loadError *LoadError + if !errors.As(err, &loadError) || len(loadError.Diagnostics()) != 1 { + t.Fatalf("invalid depth accepted: %v", err) + } + if diagnostic := loadError.Diagnostics()[0]; diagnostic.Code != CodeImportInvalid || diagnostic.Path.String() != "/configkit/imports/0/depth" { + t.Fatalf("invalid depth diagnostic = %#v", diagnostic) + } + }) + } +} + func TestImportDefaultsApplyToTransitiveAndUnderlayImports(t *testing.T) { type config struct { Main string `yaml:"main"` diff --git a/internal/engine/import_spec.go b/internal/engine/import_spec.go index 3af59dc..608f9e5 100644 --- a/internal/engine/import_spec.go +++ b/internal/engine/import_spec.go @@ -427,7 +427,8 @@ func parseImportDepth(node *document.Node) (int, bool) { } digit := uint64(character - '0') if value > (maxInt-digit)/10 { - return int(maxInt), true + value = maxInt + continue // Saturate the bound, but validate every remaining character. } value = value*10 + digit }