Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions cmd/wfctl/infra_align.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func runInfraAlign(args []string) error {
if alignExitCode(findings, opts.strict) != 0 {
var failCount int
for _, f := range findings {
if f.Severity == "FAIL" || (opts.strict && f.Severity == "WARN") {
if f.Severity == "FAIL" || f.Severity == "ERROR" || (opts.strict && f.Severity == "WARN") {
failCount++
}
}
Expand Down Expand Up @@ -235,9 +235,19 @@ func loadPlanJSON(path string) (*interfaces.IaCPlan, error) {
}

// alignExitCode returns 0 (success) or 1 (failure) based on findings and flags.
//
// Severities that always block (exit 1):
// - FAIL: deterministic failure (e.g. unresolved env var, R-A1 missing image)
// - ERROR: hard rule violation that must block deploy regardless of --strict
// (introduced in rev3 of the spaces-key plan for R-A9; ERROR is treated
// identically to FAIL by exit-code logic so a rule author can use ERROR to
// signal "this is fixable; here's the fix" without weakening the gate)
//
// Severities that block only under --strict:
// - WARN: advisory, surfaced as a heads-up but non-blocking by default
func alignExitCode(findings []AlignFinding, strict bool) int {
for _, f := range findings {
if f.Severity == "FAIL" {
if f.Severity == "FAIL" || f.Severity == "ERROR" {
return 1
}
if strict && f.Severity == "WARN" {
Expand Down Expand Up @@ -268,11 +278,13 @@ func renderAlignMarkdown(findings []AlignFinding) string {
}
sb.WriteString("\n")

var failCount, warnCount int
var failCount, errorCount, warnCount int
for _, f := range findings {
switch f.Severity {
case "FAIL":
failCount++
case "ERROR":
errorCount++
case "WARN":
warnCount++
}
Expand All @@ -282,6 +294,9 @@ func renderAlignMarkdown(findings []AlignFinding) string {
if failCount > 0 {
parts = append(parts, fmt.Sprintf("%d FAIL", failCount))
}
if errorCount > 0 {
parts = append(parts, fmt.Sprintf("%d ERROR", errorCount))
}
if warnCount > 0 {
parts = append(parts, fmt.Sprintf("%d WARN", warnCount))
}
Expand Down
33 changes: 22 additions & 11 deletions cmd/wfctl/infra_align_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ import (

// AlignFinding is a single rule violation emitted by an alignment check.
type AlignFinding struct {
Rule string // R-A1, R-A2, etc.
Severity string // FAIL or WARN
Rule string // R-A1, R-A2, etc.
// Severity is one of:
// - "FAIL" — always blocks (exit 1)
// - "ERROR" — always blocks (exit 1); used by rules that want to
// carry a fix-suggestion in the message (e.g. R-A9)
// - "WARN" — blocks only under --strict
Severity string
Resource string // affected resource name
Message string // human-readable description
}
Expand Down Expand Up @@ -686,15 +691,21 @@ func checkRA8(ctx *alignContext) []AlignFinding {

// ── R-A9: suspicious provider_credential key suffix ────────────────────────

// checkRA9 warns when a secrets.generate entry with type "provider_credential"
// uses a key that already ends with a known sub-key suffix (e.g. "_access_key",
// "_secret_key"). This pattern means the caller pre-appended the sub-key name
// that bootstrapSecrets will append again, producing double-suffixed storage
// keys such as SPACES_access_key_access_key. The canonical form is to use the
// root key (e.g. "SPACES") and let bootstrapSecrets derive the sub-key names.
// checkRA9 fires (as an ERROR) when a secrets.generate entry with type
// "provider_credential" uses a key that already ends with a known sub-key
// suffix (e.g. "_access_key", "_secret_key"). This is the doubled-create
// anti-pattern: each sub-keyed entry causes bootstrapSecrets to create a
// separate cloud credential, producing an orphaned pair of keys instead of
// a single canonical credential. The canonical form is to use the root key
// (e.g. "SPACES") and let bootstrapSecrets auto-derive the sub-key names
// from providerCredentialSubKeys[source].
//
// The rule only fires for sources registered in providerCredentialSubKeys.
// Unknown sources are skipped — we cannot predict their sub-key names.
//
// Severity: ERROR (was WARN through rev2 of the spaces-key plan; flipped to
// ERROR in rev3 so `wfctl infra align --strict` blocks deploy when the
// anti-pattern is present).
Comment on lines +707 to +708
func checkRA9(ctx *alignContext) []AlignFinding {
var findings []AlignFinding
for _, gen := range ctx.secretGens {
Expand All @@ -710,11 +721,11 @@ func checkRA9(ctx *alignContext) []AlignFinding {
if strings.HasSuffix(gen.Key, suffix) {
findings = append(findings, AlignFinding{
Rule: "R-A9",
Severity: "WARN",
Severity: "ERROR",
Resource: gen.Key,
Message: fmt.Sprintf(
Comment on lines 722 to 726
"provider_credential key %q ends with auto-generated suffix %q — use root key (e.g. %q) and let bootstrapSecrets derive sub-keys",
gen.Key, suffix, strings.TrimSuffix(gen.Key, suffix),
"provider_credential key %q ends in %q; use canonical key %q — bootstrap auto-derives the sub-keys for source %q",
gen.Key, suffix, strings.TrimSuffix(gen.Key, suffix), gen.Source,
),
Comment on lines 726 to 729
})
break // one finding per gen entry is enough
Expand Down
81 changes: 77 additions & 4 deletions cmd/wfctl/infra_align_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,41 @@ func TestInfraAlign_ExitCode_WarnStrict(t *testing.T) {
}
}

// TestAlignExitCode_ErrorSeverity_Returns1 verifies that ERROR severity
// (introduced in rev3 of the spaces-key plan for R-A9) blocks deploy with
// exit 1 even without --strict. Without this, an ERROR finding silently
// downgrades to non-blocking — defeating the rev3 requirement that the
// doubled-create anti-pattern fail CI.
func TestAlignExitCode_ErrorSeverity_Returns1(t *testing.T) {
Comment on lines +886 to +891
findings := []AlignFinding{
{Rule: "R-A9", Severity: "ERROR", Resource: "SPACES_access_key", Message: "doubled-create"},
}
if code := alignExitCode(findings, false); code != 1 {
t.Errorf("alignExitCode(ERROR, strict=false) = %d, want 1 — ERROR must always block", code)
}
if code := alignExitCode(findings, true); code != 1 {
t.Errorf("alignExitCode(ERROR, strict=true) = %d, want 1", code)
}
}

// TestAlignRender_ErrorSeverity_CountedInSummary verifies that the markdown
// summary includes ERROR alongside FAIL/WARN counts. Without this, ERROR
// findings would render in the table but be invisible in the summary line,
// hiding the deploy-blocking signal from CI consumers.
func TestAlignRender_ErrorSeverity_CountedInSummary(t *testing.T) {
findings := []AlignFinding{
{Rule: "R-A9", Severity: "ERROR", Resource: "SPACES_access_key", Message: "doubled-create"},
Comment on lines +903 to +909
{Rule: "R-A6", Severity: "WARN", Resource: "nats", Message: "advisory"},
}
out := renderAlignMarkdown(findings)
if !strings.Contains(out, "1 ERROR") {
t.Errorf("summary should report '1 ERROR', got: %s", out)
}
if !strings.Contains(out, "1 WARN") {
t.Errorf("summary should still report '1 WARN', got: %s", out)
}
}

// ── R-A9: suspicious provider_credential key ──────────────────────────────────

// TestCheckRA9_SuspiciousProviderCredentialKey is a table-driven unit test
Expand Down Expand Up @@ -939,8 +974,8 @@ func TestCheckRA9_SuspiciousProviderCredentialKey(t *testing.T) {
if tc.wantFinding && findings[0].Rule != "R-A9" {
t.Errorf("expected Rule=R-A9, got %q", findings[0].Rule)
}
if tc.wantFinding && findings[0].Severity != "WARN" {
t.Errorf("expected Severity=WARN, got %q", findings[0].Severity)
if tc.wantFinding && findings[0].Severity != "ERROR" {
t.Errorf("expected Severity=ERROR, got %q", findings[0].Severity)
Comment on lines +977 to +978
}
})
}
Expand Down Expand Up @@ -973,8 +1008,16 @@ modules:
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !findingsHaveRuleAndSeverity(findings, "R-A9", "WARN") {
t.Errorf("expected R-A9 WARN, got: %v", findings)
if !findingsHaveRuleAndSeverity(findings, "R-A9", "ERROR") {
t.Errorf("expected R-A9 ERROR, got: %v", findings)
}
// End-to-end: ERROR severity must produce a non-zero exit code regardless
// of --strict (the user-visible CI gate, not just the severity label).
if code := alignExitCode(findings, false); code != 1 {
t.Errorf("alignExitCode(strict=false) = %d, want 1 — R-A9 ERROR must block without --strict", code)
}
if code := alignExitCode(findings, true); code != 1 {
t.Errorf("alignExitCode(strict=true) = %d, want 1", code)
}
}

Expand Down Expand Up @@ -1004,6 +1047,36 @@ secrets:
}
}

// TestInfraAlign_RA9_CanonicalSingleEntry_Passes is the positive happy-path fixture for
// the R-A9 severity flip (rev3): the canonical single-entry SPACES key with
// no doubled-create anti-pattern must pass `wfctl infra align --strict` with
// exit code 0 and produce zero R-A9 findings.
//
// This is the inverse of TestInfraAlign_RA9_SuspiciousKey_Fires: it locks in
// that the rule does not regress into false positives once it fires as ERROR.
func TestInfraAlign_RA9_CanonicalSingleEntry_Passes(t *testing.T) {
yaml := `
secrets:
generate:
- key: SPACES
type: provider_credential
source: digitalocean.spaces
name: my-deploy-key
`
cfg := writeAlignYAML(t, yaml)
opts := alignOptions{configFile: cfg, strict: true}
findings, err := runInfraAlignChecks(opts)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if findingsHaveRule(findings, "R-A9") {
t.Fatalf("canonical shape should not trigger R-A9; got: %+v", findings)
}
if code := alignExitCode(findings, true); code != 0 {
t.Fatalf("canonical shape should pass --strict; got exit=%d, findings=%+v", code, findings)
}
}

func TestInfraAlign_RenderMarkdown(t *testing.T) {
findings := []AlignFinding{
{Rule: "R-A6", Severity: "WARN", Resource: "nats-broker", Message: "internal service should use expose: internal"},
Expand Down
14 changes: 10 additions & 4 deletions docs/WFCTL.md
Original file line number Diff line number Diff line change
Expand Up @@ -1437,8 +1437,14 @@ WFCTL_REFRESH_OUTPUTS=1 wfctl infra apply --auto-approve --skip-refresh -c infra
#### `infra align`

Run a battery of static alignment checks against a config (and optionally a
plan). Each rule (`R-A1` … `R-A10`) emits findings as a `FAIL` (always
non-zero exit) or `WARN` (non-zero only with `--strict`).
plan). Each rule (`R-A1` … `R-A10`) emits findings at one of three severity
tiers:

- `FAIL` — deterministic failure; always non-zero exit (e.g. unresolved env var).
- `ERROR` — hard rule violation; always non-zero exit, equivalent to `FAIL`
for exit-code purposes. Used by rules that pair the violation with a
fix-suggestion in the message (e.g. `R-A9`).
- `WARN` — advisory; non-zero exit only with `--strict`.

```
wfctl infra align [--config <file>] [--env <env>] [--plan <plan.json>] [--strict] [--strict-health] [--strict-cidr] [--max-changes N]
Expand All @@ -1449,7 +1455,7 @@ wfctl infra align [--config <file>] [--env <env>] [--plan <plan.json>] [--strict
| `--config`, `-c` | _(auto-detected)_ | Config file (searches `infra.yaml`, `config/infra.yaml`) |
| `--env` | `` | Environment name for per-env config resolution |
| `--plan` | `` | Path to a plan JSON file. Enables `R-A7` (plan-output sanity) and `R-A10` (provider `ValidatePlan` dispatch). |
| `--strict` | `false` | Treat all `WARN` findings as `FAIL` (exit 1) |
| `--strict` | `false` | Treat all `WARN` findings as failing (exit 1). `FAIL` and `ERROR` always block regardless of this flag. |
| `--strict-health` | `false` | Treat `R-A2` health-check `WARN`s as `FAIL` |
| `--strict-cidr` | `false` | Enable strict CIDR overlap checks (reserved) |
| `--max-changes` | `50` | Warn when the plan has more than N actions |
Expand All @@ -1464,7 +1470,7 @@ wfctl infra align [--config <file>] [--env <env>] [--plan <plan.json>] [--strict
| R-A6 | Network/exposure alignment | FAIL or WARN |
| R-A7 | Plan-output sanity (requires `--plan`) | FAIL or WARN |
| R-A8 | WebAuthn RP_ID alignment | FAIL |
| R-A9 | Suspicious `provider_credential` key suffix | WARN |
| R-A9 | Suspicious `provider_credential` key suffix (doubled-create anti-pattern) | ERROR |
| R-A10 | Provider `ValidatePlan` diagnostics (requires `--plan`) | FAIL or WARN |

**R-A10 — provider-side cross-resource validation.** When `--plan` is given,
Expand Down
Loading