Skip to content

Commit e081b71

Browse files
Copilotintel352
andauthored
wfctl validate: add field-level cross-reference checking for step outputs, SQL aliases, and plain-string step refs (#373)
* Initial plan * feat: add cross-reference checking for step output fields, SQL aliases, and plain-string step refs Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/70dc13a0-5cff-4fcf-bf94-6417d19991cf * fix: skip field-path validation for steps with dynamic/wildcard placeholder outputs Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/bccc8889-cac7-4453-9778-c5596f9372b9 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Co-authored-by: Jonathan Langevin <codingsloth@pm.me>
1 parent 4745a3a commit e081b71

6 files changed

Lines changed: 612 additions & 56 deletions

File tree

cmd/wfctl/infra_state.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,11 @@ func runInfraStateImport(args []string) error {
181181
// --- tfstate export ---
182182

183183
type tfState struct {
184-
Version int `json:"version"`
185-
TerraformVersion string `json:"terraform_version"`
186-
Serial int `json:"serial"`
187-
Lineage string `json:"lineage"`
188-
Outputs map[string]any `json:"outputs"`
184+
Version int `json:"version"`
185+
TerraformVersion string `json:"terraform_version"`
186+
Serial int `json:"serial"`
187+
Lineage string `json:"lineage"`
188+
Outputs map[string]any `json:"outputs"`
189189
Resources []tfStateResource `json:"resources"`
190190
}
191191

@@ -304,11 +304,11 @@ func importFromPulumi(srcFile, stateDir string) error {
304304
var checkpoint struct {
305305
Latest struct {
306306
Resources []struct {
307-
URN string `json:"urn"`
308-
Type string `json:"type"`
309-
ID string `json:"id"`
310-
Inputs map[string]any `json:"inputs"`
311-
Outputs map[string]any `json:"outputs"`
307+
URN string `json:"urn"`
308+
Type string `json:"type"`
309+
ID string `json:"id"`
310+
Inputs map[string]any `json:"inputs"`
311+
Outputs map[string]any `json:"outputs"`
312312
} `json:"resources"`
313313
} `json:"latest"`
314314
}

cmd/wfctl/main.go

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,30 +53,30 @@ func isHelpRequested(err error) bool {
5353
// the runtime functions that are registered in the CLICommandRegistry service
5454
// and invoked by step.cli_invoke from within each command's pipeline.
5555
var commands = map[string]func([]string) error{
56-
"init": runInit,
57-
"validate": runValidate,
58-
"inspect": runInspect,
59-
"run": runRun,
60-
"plugin": runPlugin,
61-
"pipeline": runPipeline,
62-
"schema": runSchema,
63-
"snippets": runSnippets,
64-
"manifest": runManifest,
65-
"migrate": runMigrate,
66-
"build-ui": runBuildUI,
67-
"ui": runUI,
68-
"publish": runPublish,
69-
"deploy": runDeploy,
70-
"api": runAPI,
71-
"diff": runDiff,
72-
"template": runTemplate,
73-
"contract": runContract,
74-
"compat": runCompat,
75-
"generate": runGenerate,
76-
"git": runGit,
77-
"registry": runRegistry,
78-
"update": runUpdate,
79-
"mcp": runMCP,
56+
"init": runInit,
57+
"validate": runValidate,
58+
"inspect": runInspect,
59+
"run": runRun,
60+
"plugin": runPlugin,
61+
"pipeline": runPipeline,
62+
"schema": runSchema,
63+
"snippets": runSnippets,
64+
"manifest": runManifest,
65+
"migrate": runMigrate,
66+
"build-ui": runBuildUI,
67+
"ui": runUI,
68+
"publish": runPublish,
69+
"deploy": runDeploy,
70+
"api": runAPI,
71+
"diff": runDiff,
72+
"template": runTemplate,
73+
"contract": runContract,
74+
"compat": runCompat,
75+
"generate": runGenerate,
76+
"git": runGit,
77+
"registry": runRegistry,
78+
"update": runUpdate,
79+
"mcp": runMCP,
8080
"modernize": runModernize,
8181
"infra": runInfra,
8282
"docs": runDocs,

cmd/wfctl/plugin_install_new_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ func buildPluginTarGz(t *testing.T, pluginName string, binaryContent []byte, pjC
2424
t.Helper()
2525
topDir := pluginName + "-" + runtime.GOOS + "-" + runtime.GOARCH
2626
entries := map[string][]byte{
27-
topDir + "/" + pluginName: binaryContent,
28-
topDir + "/plugin.json": pjContent,
27+
topDir + "/" + pluginName: binaryContent,
28+
topDir + "/plugin.json": pjContent,
2929
}
3030
return buildTarGz(t, entries, 0755)
3131
}
@@ -166,8 +166,8 @@ func TestInstallFromURL_NameNormalization(t *testing.T) {
166166

167167
pjContent := minimalPluginJSON(fullName, "0.1.0")
168168
entries := map[string][]byte{
169-
"top/" + fullName: []byte("#!/bin/sh\n"),
170-
"top/plugin.json": pjContent,
169+
"top/" + fullName: []byte("#!/bin/sh\n"),
170+
"top/plugin.json": pjContent,
171171
}
172172
tarball := buildTarGz(t, entries, 0755)
173173

cmd/wfctl/template_validate.go

Lines changed: 178 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,10 @@ var _ fs.FS = templateFS
565565
// templateExprRe matches template actions {{ ... }}.
566566
var templateExprRe = regexp.MustCompile(`\{\{(.*?)\}\}`)
567567

568-
// stepRefDotRe matches .steps.STEP_NAME patterns (dot access).
569-
var stepRefDotRe = regexp.MustCompile(`\.steps\.([a-zA-Z_][a-zA-Z0-9_-]*)`)
568+
// stepRefDotRe matches .steps.STEP_NAME and captures an optional field path.
569+
// Group 1: step name (may contain hyphens).
570+
// Group 2: remaining dot-path (e.g. ".row.auth_token"), field names without hyphens.
571+
var stepRefDotRe = regexp.MustCompile(`\.steps\.([a-zA-Z_][a-zA-Z0-9_-]*)((?:\.[a-zA-Z_][a-zA-Z0-9_]*)*)`)
570572

571573
// stepRefIndexRe matches index .steps "STEP_NAME" patterns.
572574
var stepRefIndexRe = regexp.MustCompile(`index\s+\.steps\s+"([^"]+)"`)
@@ -579,11 +581,66 @@ var stepRefFuncRe = regexp.MustCompile(`(?:^|\||\()\s*step\s+"([^"]+)"`)
579581
// including continuation segments after the hyphenated part.
580582
var hyphenDotRe = regexp.MustCompile(`\.[a-zA-Z_][a-zA-Z0-9_]*-[a-zA-Z0-9_-]*(?:\.[a-zA-Z_][a-zA-Z0-9_-]*)*`)
581583

584+
// plainStepPathRe matches bare step context-key references such as
585+
// "steps.STEP_NAME.field.subfield" used in plain-string config values (no {{ }}).
586+
var plainStepPathRe = regexp.MustCompile(`^steps\.([a-zA-Z_][a-zA-Z0-9_-]*)((?:\.[a-zA-Z_][a-zA-Z0-9_]*)*)`)
587+
588+
// stepBuildInfo holds the type and config of a pipeline step, used for output field validation.
589+
type stepBuildInfo struct {
590+
stepType string
591+
stepConfig map[string]any
592+
}
593+
594+
// dbQueryStepTypes is the set of step types that produce a "row" or "rows" output
595+
// from a SQL query and support SQL alias extraction.
596+
var dbQueryStepTypes = map[string]bool{
597+
"step.db_query": true,
598+
"step.db_query_cached": true,
599+
}
600+
601+
// isDBQueryStep reports whether a step type is a DB query step.
602+
func isDBQueryStep(t string) bool { return dbQueryStepTypes[t] }
603+
604+
// joinOutputKeys returns a comma-joined list of output key names for error messages,
605+
// omitting placeholder/wildcard entries like "(key)", "(dynamic)", "(nested)".
606+
func joinOutputKeys(outputs []schema.InferredOutput) string {
607+
keys := make([]string, 0, len(outputs))
608+
for _, o := range outputs {
609+
if !isPlaceholderOutputKey(o.Key) {
610+
keys = append(keys, o.Key)
611+
}
612+
}
613+
return strings.Join(keys, ", ")
614+
}
615+
616+
// isPlaceholderOutputKey reports whether an output key is a dynamic/wildcard
617+
// placeholder (e.g. "(key)", "(dynamic)", "(nested)"). Steps that expose
618+
// such placeholders produce outputs whose field names cannot be statically
619+
// determined, so field-path validation should be skipped for them.
620+
func isPlaceholderOutputKey(key string) bool {
621+
return len(key) >= 2 && key[0] == '(' && key[len(key)-1] == ')'
622+
}
623+
624+
// hasDynamicOutputs reports whether any output in the list is a wildcard
625+
// placeholder, meaning the step emits fields that are not statically known.
626+
func hasDynamicOutputs(outputs []schema.InferredOutput) bool {
627+
for _, o := range outputs {
628+
if isPlaceholderOutputKey(o.Key) {
629+
return true
630+
}
631+
}
632+
return false
633+
}
634+
582635
// validatePipelineTemplates checks template expressions in pipeline step configs for
583636
// references to nonexistent or forward-declared steps and common template pitfalls.
584637
func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *templateValidationResult) {
585-
// Build ordered step name list
586-
stepNames := make(map[string]int) // step name -> index in pipeline
638+
// Build ordered step name list and per-step type/config info.
639+
stepNames := make(map[string]int) // step name -> index in pipeline
640+
stepInfos := make(map[string]stepBuildInfo) // step name -> type and config
641+
642+
reg := schema.NewStepSchemaRegistry()
643+
587644
for i, stepRaw := range stepsRaw {
588645
stepMap, ok := stepRaw.(map[string]any)
589646
if !ok {
@@ -592,6 +649,12 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp
592649
name, _ := stepMap["name"].(string)
593650
if name != "" {
594651
stepNames[name] = i
652+
sType, _ := stepMap["type"].(string)
653+
sCfg, _ := stepMap["config"].(map[string]any)
654+
if sCfg == nil {
655+
sCfg = map[string]any{}
656+
}
657+
stepInfos[name] = stepBuildInfo{stepType: sType, stepConfig: sCfg}
595658
}
596659
}
597660

@@ -624,25 +687,29 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp
624687
continue
625688
}
626689

627-
// Check for step name references via dot-access
690+
// Check for step name references via dot-access (captures optional field path)
628691
dotMatches := stepRefDotRe.FindAllStringSubmatch(actionContent, -1)
629692
for _, m := range dotMatches {
630693
refName := m[1]
631-
validateStepRef(pipelineName, stepName, refName, i, stepNames, result)
694+
fieldPath := ""
695+
if len(m) > 2 {
696+
fieldPath = m[2]
697+
}
698+
validateStepRef(pipelineName, stepName, refName, fieldPath, i, stepNames, stepInfos, reg, result)
632699
}
633700

634-
// Check for step name references via index
701+
// Check for step name references via index (no field path resolvable)
635702
indexMatches := stepRefIndexRe.FindAllStringSubmatch(actionContent, -1)
636703
for _, m := range indexMatches {
637704
refName := m[1]
638-
validateStepRef(pipelineName, stepName, refName, i, stepNames, result)
705+
validateStepRef(pipelineName, stepName, refName, "", i, stepNames, stepInfos, reg, result)
639706
}
640707

641-
// Check for step name references via step function
708+
// Check for step name references via step function (no field path resolvable)
642709
funcMatches := stepRefFuncRe.FindAllStringSubmatch(actionContent, -1)
643710
for _, m := range funcMatches {
644711
refName := m[1]
645-
validateStepRef(pipelineName, stepName, refName, i, stepNames, result)
712+
validateStepRef(pipelineName, stepName, refName, "", i, stepNames, stepInfos, reg, result)
646713
}
647714

648715
// Warn on hyphenated dot-access (auto-fixed but suggest preferred syntax)
@@ -652,23 +719,122 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp
652719
}
653720
}
654721
}
722+
723+
// Validate plain-string step references in specific config fields
724+
// (e.g. secret_from, backend_url_key, field in conditional/branch).
725+
if stepCfg, ok := stepMap["config"].(map[string]any); ok {
726+
validatePlainStepRefs(pipelineName, stepName, i, stepCfg, stepNames, stepInfos, reg, result)
727+
}
655728
}
656729
}
657730

658731
// validateStepRef checks that a referenced step name exists and appears before the
659-
// current step in the pipeline execution order.
660-
func validateStepRef(pipelineName, currentStep, refName string, currentIdx int, stepNames map[string]int, result *templateValidationResult) {
732+
// current step in the pipeline execution order. When fieldPath is non-empty it
733+
// also validates the first output field name against the step's known outputs, and
734+
// for db_query steps it performs best-effort SQL alias checking for "row.<col>" paths.
735+
func validateStepRef(pipelineName, currentStep, refName, fieldPath string, currentIdx int, stepNames map[string]int, stepInfos map[string]stepBuildInfo, reg *schema.StepSchemaRegistry, result *templateValidationResult) {
661736
refIdx, exists := stepNames[refName]
662737
switch {
663738
case !exists:
664739
result.Warnings = append(result.Warnings,
665740
fmt.Sprintf("pipeline %q step %q: references step %q which does not exist in this pipeline", pipelineName, currentStep, refName))
741+
return
666742
case refIdx == currentIdx:
667743
result.Warnings = append(result.Warnings,
668744
fmt.Sprintf("pipeline %q step %q: references itself; a step cannot use its own outputs because they are not available until after execution", pipelineName, currentStep))
745+
return
669746
case refIdx > currentIdx:
670747
result.Warnings = append(result.Warnings,
671748
fmt.Sprintf("pipeline %q step %q: references step %q which has not executed yet (appears later in pipeline)", pipelineName, currentStep, refName))
749+
return
750+
}
751+
752+
// Step exists and precedes the current step — validate the output field path.
753+
if fieldPath == "" {
754+
return
755+
}
756+
757+
info, ok := stepInfos[refName]
758+
if !ok || info.stepType == "" {
759+
return
760+
}
761+
762+
outputs := reg.InferStepOutputs(info.stepType, info.stepConfig)
763+
if len(outputs) == 0 {
764+
return // no schema information available; skip
765+
}
766+
767+
// If any output key is a placeholder (e.g. "(key)", "(dynamic)", "(nested)"),
768+
// the step emits dynamic fields whose names cannot be statically determined.
769+
// Skip field-path validation for such steps to avoid false positives.
770+
if hasDynamicOutputs(outputs) {
771+
return
772+
}
773+
774+
// Split ".row.auth_token" → ["row", "auth_token"]
775+
parts := strings.Split(strings.TrimPrefix(fieldPath, "."), ".")
776+
if len(parts) == 0 || parts[0] == "" {
777+
return
778+
}
779+
firstField := parts[0]
780+
781+
// Check the first field against known output keys.
782+
var matchedOutput *schema.InferredOutput
783+
for i := range outputs {
784+
if outputs[i].Key == firstField {
785+
matchedOutput = &outputs[i]
786+
break
787+
}
788+
}
789+
if matchedOutput == nil {
790+
result.Warnings = append(result.Warnings,
791+
fmt.Sprintf("pipeline %q step %q: references step %q output field %q which is not a known output of step type %q (known outputs: %s)",
792+
pipelineName, currentStep, refName, firstField, info.stepType, joinOutputKeys(outputs)))
793+
return
794+
}
795+
796+
// For db_query/db_query_cached steps, try SQL alias validation on "row.<col>" paths.
797+
if firstField == "row" && len(parts) > 1 && isDBQueryStep(info.stepType) {
798+
columnName := parts[1]
799+
query, _ := info.stepConfig["query"].(string)
800+
if query != "" {
801+
sqlCols := extractSQLColumns(query)
802+
if len(sqlCols) > 0 {
803+
found := false
804+
for _, col := range sqlCols {
805+
if col == columnName {
806+
found = true
807+
break
808+
}
809+
}
810+
if !found {
811+
result.Warnings = append(result.Warnings,
812+
fmt.Sprintf("pipeline %q step %q: references step %q output field \"row.%s\" but the SQL query does not select column %q (available: %s)",
813+
pipelineName, currentStep, refName, columnName, columnName, strings.Join(sqlCols, ", ")))
814+
}
815+
}
816+
}
817+
}
818+
}
819+
820+
// validatePlainStepRefs checks plain-string config values that contain bare step
821+
// context-key references (e.g. "steps.STEP_NAME.field") in config fields known to
822+
// accept such paths: secret_from, backend_url_key, and field (conditional/branch).
823+
func validatePlainStepRefs(pipelineName, stepName string, stepIdx int, stepCfg map[string]any, stepNames map[string]int, stepInfos map[string]stepBuildInfo, reg *schema.StepSchemaRegistry, result *templateValidationResult) {
824+
// Config keys that are documented to accept a bare "steps.X.y" context path.
825+
plainRefKeys := []string{"secret_from", "backend_url_key", "field"}
826+
for _, key := range plainRefKeys {
827+
val, ok := stepCfg[key].(string)
828+
if !ok || val == "" {
829+
continue
830+
}
831+
m := plainStepPathRe.FindStringSubmatch(val)
832+
if m == nil {
833+
continue
834+
}
835+
refName := m[1]
836+
fieldPath := m[2] // already in ".field.subfield" form
837+
validateStepRef(pipelineName, stepName, refName, fieldPath, stepIdx, stepNames, stepInfos, reg, result)
672838
}
673839
}
674840

0 commit comments

Comments
 (0)