fix(migrations): parseMigrationStatus accepts fresh-database output without explicit Dirty line

.golangci.yml

@@ -27,6 +27,7 @@ linters:
         - G104 # Duplicates errcheck
         - G304 # File inclusion from variable - expected for config/dynamic loading
         - G704 # SSRF via taint analysis - URLs from admin config are intentional
+        - G710 # Open redirect via taint - OAuth2 redirect targets are provider-issued URLs from admin-configured provider endpoints
         - G118 # Context cancel/goroutine patterns - cancel is returned to caller or used in signal handler; background goroutines are intentional
         - G120 # Form data size - ParseMultipartForm already sets explicit limits; FormValue after that is already bounded
         - G122 # WalkDir/Walk TOCTOU - OS-provided walk paths; acceptable for CI/CD pipeline tooling
@@ -53,6 +54,14 @@ linters:
         linters:
           - staticcheck
         text: "SA5011"
+      # Third-party vendored Go code in ui/node_modules is not ours to fix
+      - path: ui/node_modules/
+        linters:
+          - govet
+          - staticcheck
+          - unused
+          - gocritic
+          - gosec
     presets:
       - std-error-handling
 

cmd/iac-codemod/add_validate_plan.go

@@ -12,7 +12,6 @@ import (
 	"go/token"
 	"io"
 	"io/fs"
-	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -643,50 +642,6 @@ func qualifierFromProviderMethods(methods []*ast.FuncDecl) string {
 	return ""
 }
 
-// siblingUsesInterfacesImport returns true if any non-test .go file
-// in dir (other than excludePath) imports
-// github.com/GoCodeAlone/workflow/interfaces. Used to decide whether
-// to inject an interfaces import into a file that doesn't have one
-// when emitting a qualified ValidatePlan stub (review round-4 #1).
-func siblingUsesInterfacesImport(dir, excludePath string) bool {
-	const wantPath = "github.com/GoCodeAlone/workflow/interfaces"
-	entries, err := os.ReadDir(dir)
-	if err != nil {
-		return false
-	}
-	for _, e := range entries {
-		if e.IsDir() {
-			continue
-		}
-		name := e.Name()
-		if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
-			continue
-		}
-		fpath := filepath.Join(dir, name)
-		if fpath == excludePath {
-			continue
-		}
-		src, err := readFile(fpath)
-		if err != nil {
-			continue
-		}
-		fs := token.NewFileSet()
-		sib, err := parser.ParseFile(fs, fpath, src, parser.ImportsOnly)
-		if err != nil {
-			continue
-		}
-		for _, imp := range sib.Imports {
-			if imp.Path == nil {
-				continue
-			}
-			if strings.Trim(imp.Path.Value, `"`) == wantPath {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 // interfacesQualifier returns the package alias `file` uses for
 // github.com/GoCodeAlone/workflow/interfaces. If the import is
 // renamed (`alias "github.com/.../interfaces"`), the alias name is

cmd/iac-codemod/lint.go

@@ -1039,39 +1039,6 @@ func receiverTypeName(fn *ast.FuncDecl) string {
 	return id.Name
 }
 
-// bodyCallsSelector reports whether the function body contains a
-// CallExpr whose callee is a SelectorExpr with the given X.Name and
-// Sel.Name, e.g. `wfctlhelpers.Plan(...)`.
-func bodyCallsSelector(body *ast.BlockStmt, pkgIdent, selName string) bool {
-	if body == nil {
-		return false
-	}
-	found := false
-	ast.Inspect(body, func(n ast.Node) bool {
-		if found {
-			return false
-		}
-		call, ok := n.(*ast.CallExpr)
-		if !ok {
-			return true
-		}
-		sel, ok := call.Fun.(*ast.SelectorExpr)
-		if !ok {
-			return true
-		}
-		x, ok := sel.X.(*ast.Ident)
-		if !ok {
-			return true
-		}
-		if x.Name == pkgIdent && sel.Sel.Name == selName {
-			found = true
-			return false
-		}
-		return true
-	})
-	return found
-}
-
 // bodyReferencesField reports whether the function body references any
 // SelectorExpr with the given Sel.Name, e.g. any `<X>.ForceNew`.
 func bodyReferencesField(body *ast.BlockStmt, fieldName string) bool {

cmd/iac-codemod/refactor_apply.go

@@ -22,10 +22,6 @@ func init() {
 	modes["refactor-apply"] = runRefactorApply
 }
 
-// applyCanonicalCallExpr is the canonical replacement-body expression
-// emitted by refactor-apply.
-const applyCanonicalCallExpr = "wfctlhelpers.ApplyPlan(ctx, p, plan)"
-
 // applyClassification labels the disposition of a single Apply()
 // method site. The non-canonical idioms are surfaced as distinct
 // classes so the report can suggest the right hand-port handling.
@@ -123,7 +119,7 @@ func runRefactorApply(args []string, opts *Options, stdout, stderr io.Writer) in
 	if *reportFile != "" {
 		var buf bytes.Buffer
 		report.print(&buf, opts)
-		if err := os.WriteFile(*reportFile, buf.Bytes(), 0o644); err != nil {
+		if err := os.WriteFile(*reportFile, buf.Bytes(), 0o600); err != nil {
 			fmt.Fprintf(stderr, "iac-codemod refactor-apply: write report-file %s: %v\n", *reportFile, err)
 			return 1
 		}
@@ -476,7 +472,7 @@ func isCanonicalApplyLoopAssign(a *ast.AssignStmt, recvName string) bool {
 	if !ok {
 		return false
 	}
-	if !((recvName != "" && x.Name == recvName) || (recvName == "" && x.Name == "p")) {
+	if (recvName == "" || x.Name != recvName) && (recvName != "" || x.Name != "p") {
 		return false
 	}
 	// Round-12 #7: also verify the lookup KEY is `action.Resource.Type`.
@@ -1162,7 +1158,7 @@ func isProviderIDGuard(ifs *ast.IfStmt) bool {
 	if id, ok := be.Y.(*ast.Ident); ok && id.Name == "nil" {
 		yIsNil = true
 	}
-	if !(xIsCurrent && yIsNil) {
+	if !xIsCurrent || !yIsNil {
 		// Allow the reverse order too (`nil != action.Current`),
 		// though it's not idiomatic Go.
 		yIsCurrent := false
@@ -1173,7 +1169,7 @@ func isProviderIDGuard(ifs *ast.IfStmt) bool {
 		if id, ok := be.X.(*ast.Ident); ok && id.Name == "nil" {
 			xIsNil = true
 		}
-		if !(yIsCurrent && xIsNil) {
+		if !yIsCurrent || !xIsNil {
 			return false
 		}
 	}

cmd/iac-codemod/refactor_plan.go

@@ -39,11 +39,6 @@ const helperImportPath = "github.com/GoCodeAlone/workflow/iac/wfctlhelpers"
 // files would fail to compile".
 const planHelperImportPath = "github.com/GoCodeAlone/workflow/platform"
 
-// planCanonicalCallExpr is the canonical replacement-body expression
-// emitted by refactor-plan. Calls platform.ComputePlan (the real helper);
-// see planHelperImportPath above for the review-correction rationale.
-const planCanonicalCallExpr = "platform.ComputePlan(ctx, p, desired, current)"
-
 // planClassification labels the disposition of a single Plan() method
 // site. Each report entry carries one classification; the rewriter
 // honors only `planCanonical`.
@@ -1108,13 +1103,13 @@ func rewritePlanBody(fn *ast.FuncDecl, file *ast.File) {
 			ast.NewIdent(currentName),
 		},
 	}
-	// plan, err := platform.ComputePlan(ctx, p, desired, current)
+	// generates: plan, err := platform.ComputePlan(ctx, p, desired, current)
 	planAssign := &ast.AssignStmt{
 		Lhs: []ast.Expr{ast.NewIdent("plan"), ast.NewIdent("err")},
 		Tok: token.DEFINE,
 		Rhs: []ast.Expr{call},
 	}
-	// return &plan, err
+	// generates: return &plan, err
 	returnStmt := &ast.ReturnStmt{
 		Results: []ast.Expr{
 			&ast.UnaryExpr{Op: token.AND, X: ast.NewIdent("plan")},

cmd/wfctl/infra.go

@@ -295,7 +295,7 @@ func runInfraPlan(args []string) error {
 			// cmd/wfctl/main.go's top-level wrapper. errors.New (rather than
 			// fmt.Errorf) avoids govet's no-verbs noise and is canonical for
 			// fixed-string error literals per Go convention.
-			return errors.New("this plan requires JIT resolution; persisted plan.json is not supported. Run 'wfctl infra apply' directly without -o/--plan.")
+			return errors.New("this plan requires JIT resolution; persisted plan.json is not supported. Run 'wfctl infra apply' directly without -o/--plan")
 		}
 		// Embed a hash of the desired-state inputs so wfctl infra apply --plan
 		// can detect stale plans when the config changes after plan generation.

cmd/wfctl/infra_bootstrap.go

@@ -429,13 +429,9 @@ func bootstrapSecrets(ctx context.Context, provider secrets.Provider, cfg *Secre
 		if forceRotate[gen.Key] {
 			deleteKey := gen.Key
 			if delErr := provider.Delete(ctx, deleteKey); delErr != nil {
-				if errors.Is(delErr, secrets.ErrNotFound) || errors.Is(delErr, secrets.ErrUnsupported) {
-					// Secret was already absent or provider doesn't support Delete.
-					// Log a warning and continue — the create path handles this.
-					fmt.Fprintf(os.Stderr, "warn: rotate-pre-delete %s: %v (continuing)\n", deleteKey, delErr)
-				} else {
-					fmt.Fprintf(os.Stderr, "warn: rotate-pre-delete %s: %v (continuing)\n", deleteKey, delErr)
-				}
+				// Log and continue regardless of the error kind — both absent/unsupported
+				// secrets and unexpected errors should not block the rotation flow.
+				fmt.Fprintf(os.Stderr, "warn: rotate-pre-delete %s: %v (continuing)\n", deleteKey, delErr)
 			}
 			// Invalidate the List cache so the existence check below reflects the
 			// deletion. A fresh List call after Delete is the safe path for

cmd/wfctl/infra_plan_jit_reject_test.go

@@ -20,7 +20,7 @@ import (
 // prefix from cmd/wfctl/main.go's top-level error wrapper; the value
 // returned by runInfraPlan (and asserted here) does NOT include that
 // prefix.
-const expectedJITRejectError = "this plan requires JIT resolution; persisted plan.json is not supported. Run 'wfctl infra apply' directly without -o/--plan."
+const expectedJITRejectError = "this plan requires JIT resolution; persisted plan.json is not supported. Run 'wfctl infra apply' directly without -o/--plan"
 
 // TestInfraPlan_RejectsPersistedJITPlan_WithExactErrorString is the
 // canonical T5.5 scenario: a config whose env_vars carry a

cmd/wfctl/migrations_baseline.go

@@ -230,6 +230,13 @@ func parseMigrationStatus(stdout string) (migrationBaselineCandidateResult, erro
 			pendingSeen = true
 			if line == "No migrations applied." {
 				currentSeen = true
+				// A fresh database with no migrations applied cannot be dirty.
+				// Set dirtySeen so callers don't require a redundant "Dirty: false" line
+				// in output from drivers (e.g. atlas) that omit it on a clean first run.
+				if !dirtySeen {
+					dirtySeen = true
+					// status.Dirty is already false (zero value)
+				}
 			}
 		}
 	}

cmd/wfctl/migrations_baseline_test.go

@@ -249,6 +249,62 @@ func TestParseMigrationStatusRejectsUnknownOutput(t *testing.T) {
 	}
 }
 
+func TestParseMigrationStatusFreshDatabaseNoMigrationsApplied(t *testing.T) {
+	// A fresh database with no migrations applied cannot be dirty. Drivers such as
+	// atlas omit the "Dirty:" line in this case; parseMigrationStatus must not
+	// require it when "No migrations applied." is the reported state.
+	status, err := parseMigrationStatus("No migrations applied.\n")
+	if err != nil {
+		t.Fatalf("fresh-database status without Dirty line: %v", err)
+	}
+	if status.Dirty {
+		t.Fatal("fresh-database status: expected dirty=false")
+	}
+	if status.Current != "" {
+		t.Fatalf("fresh-database status: expected empty current, got %q", status.Current)
+	}
+	if len(status.Pending) != 0 {
+		t.Fatalf("fresh-database status: expected no pending, got %v", status.Pending)
+	}
+
+	// Explicit "Dirty: false" alongside "No migrations applied." must also work.
+	status, err = parseMigrationStatus("No migrations applied.\nDirty: false\n")
+	if err != nil {
+		t.Fatalf("fresh-database status with explicit Dirty: false: %v", err)
+	}
+	if status.Dirty {
+		t.Fatal("expected dirty=false")
+	}
+
+	// "Dirty: true" with "No migrations applied." is unusual but must be respected
+	// when the driver explicitly signals it (edge case: corrupted revisions table).
+	status, err = parseMigrationStatus("No migrations applied.\nDirty: true\n")
+	if err != nil {
+		t.Fatalf("fresh-database status with explicit Dirty: true: %v", err)
+	}
+	if !status.Dirty {
+		t.Fatal("expected dirty=true when driver explicitly reports it")
+	}
+
+	// JSON format: fresh database with empty current and no pending.
+	status, err = parseMigrationStatus(`{"current":"","dirty":false,"pending":[]}`)
+	if err != nil {
+		t.Fatalf("fresh-database JSON status: %v", err)
+	}
+	if status.Dirty || status.Current != "" || len(status.Pending) != 0 {
+		t.Fatalf("unexpected fresh-database JSON status: %+v", status)
+	}
+
+	// JSON format: null pending (some drivers emit null instead of []).
+	status, err = parseMigrationStatus(`{"current":"","dirty":false,"pending":null}`)
+	if err != nil {
+		t.Fatalf("fresh-database JSON status with null pending: %v", err)
+	}
+	if status.Dirty || status.Current != "" || len(status.Pending) != 0 {
+		t.Fatalf("unexpected fresh-database JSON status (null pending): %+v", status)
+	}
+}
+
 func TestMigrationSourceChangedNormalizesDotSlashAndRoot(t *testing.T) {
 	if !migrationSourceChanged("./migrations", []string{"migrations/202604270001_add_users.up.sql"}) {
 		t.Fatal("expected ./migrations to match git diff path")
@@ -454,6 +510,66 @@ func TestExtractTarRejectsCleanedTargetOutsideDestination(t *testing.T) {
 	}
 }
 
+func TestRunMigrationsValidateAtlasFreshDatabaseStatusWithoutDirtyLine(t *testing.T) {
+	// Simulates the atlas driver returning "No migrations applied." without a
+	// "Dirty:" line after running `up` on a fresh (empty) ephemeral database.
+	// This is the scenario surfaced by the atlas executor panic fix: once the
+	// binary no longer panics, wfctl must parse the status output correctly.
+	cfgPath := writeMigrationBaselineConfigData(t, `
+version: 1
+ci:
+  migrations:
+    - name: app
+      driver: atlas
+      source_dir: migrations
+      database:
+        env: DATABASE_URL
+      validation:
+        baseline_candidate: true
+`)
+	resultPath := filepath.Join(t.TempDir(), "result.json")
+	t.Setenv("DATABASE_URL", "postgres://secret@example/db")
+
+	var calls []string
+	restore := stubMigrationBaselineHooks(t, &calls, []string{"migrations/202604270001_add_users.up.sql"}, "abc123")
+	defer restore()
+	oldRunner := newMigrationPluginRunner
+	newMigrationPluginRunner = func() migrationPluginRunner {
+		return migrationPluginRunner{
+			exec: func(_ context.Context, _ string, args []string, _ map[string]string) (migrationCommandResult, error) {
+				command := migrationCommandFromArgs(args)
+				if command == "status" {
+					// Atlas driver output for a fresh database: no "Dirty:" line.
+					return migrationCommandResult{Stdout: "No migrations applied.\n"}, nil
+				}
+				return migrationCommandResult{}, nil
+			},
+		}
+	}
+	defer func() { newMigrationPluginRunner = oldRunner }()
+
+	if err := runMigrations([]string{"validate", "--config", cfgPath, "--env", "ci", "--result-file", resultPath}); err != nil {
+		t.Fatalf("validation failed for fresh-database atlas status: %v", err)
+	}
+	data, err := os.ReadFile(resultPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var got migrationValidationResult
+	if err := json.Unmarshal(data, &got); err != nil {
+		t.Fatalf("decode result file: %v\n%s", err, data)
+	}
+	if got.Decision != "pass" || len(got.Migrations) != 1 {
+		t.Fatalf("unexpected validation result: %+v", got)
+	}
+	if got.Migrations[0].BaselineCandidate != "pass" {
+		t.Fatalf("baseline_candidate check should pass: %+v", got.Migrations[0])
+	}
+	if got.Migrations[0].Dirty {
+		t.Fatal("fresh database should not be dirty")
+	}
+}
+
 func TestRunMigrationsValidateUsesEphemeralDSNForBaselineCandidate(t *testing.T) {
 	cfgPath := writeMigrationBaselineConfig(t, true)
 	t.Setenv("DATABASE_URL", "postgres://real-db.example/app")

iac/diffcache/cache_filesystem.go

@@ -123,7 +123,7 @@ func (c *filesystemCache) Get(k Key) (interfaces.DiffResult, bool) {
 // working" signal from elsewhere. The cache-as-amortization framing
 // in the package godoc sets the expectation.
 func (c *filesystemCache) Put(k Key, result interfaces.DiffResult) {
-	if err := os.MkdirAll(c.dir, 0o755); err != nil {
+	if err := os.MkdirAll(c.dir, 0o750); err != nil {
 		return
 	}
 	env := envelope{SchemaVersion: cacheSchemaVersion, Result: result}