From 4f49c8adc431a3e88bbe1bd813622ae147d69c19 Mon Sep 17 00:00:00 2001 From: Stefan Tomm Date: Thu, 2 Jul 2026 09:01:51 +0200 Subject: [PATCH 1/3] fix: stop baking ephemeral run token into backend config OpenTofu embeds the backend config verbatim into any saved plan, so a Bearer header baked into the backend block would leak into saved plans and could be replayed long after the token is revoked (a predecessor plan may now be applied days/weeks after the DETECT run that created it). Supply the run token via TF_HTTP_USERNAME/TF_HTTP_PASSWORD instead, which OpenTofu re-reads fresh at every plan/apply. meshfed-api's TfStateRunTokenBasicAuthFilter recognizes the fixed sentinel username and rewrites the Basic password back to a Bearer token for the state endpoint. --- .../tfrun/testdata/backend.golden.tf | 3 -- tf-block-runner/tfrun/tfcmd.go | 32 ++++++++++-- tf-block-runner/tfrun/tfcmd_test.go | 51 +++++++++++++++++-- 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/tf-block-runner/tfrun/testdata/backend.golden.tf b/tf-block-runner/tfrun/testdata/backend.golden.tf index b454d6a..bc1871e 100644 --- a/tf-block-runner/tfrun/testdata/backend.golden.tf +++ b/tf-block-runner/tfrun/testdata/backend.golden.tf @@ -1,8 +1,5 @@ terraform { backend "http" { address = "https://meshstack.example.com/api/terraform/state/workspace/test-workspace/buildingBlock/test-bb-id" - headers = { - Authorization = "Bearer ephemeral-run-token" - } } } diff --git a/tf-block-runner/tfrun/tfcmd.go b/tf-block-runner/tfrun/tfcmd.go index 618af72..7b61724 100644 --- a/tf-block-runner/tfrun/tfcmd.go +++ b/tf-block-runner/tfrun/tfcmd.go @@ -26,6 +26,13 @@ const ( defaultStateFilename = "terraform.tfstate" HINT_INIT_FAILED = "Check provider and / or backend config" + + // MeshStackRunTokenBasicUser is the Basic-auth username sent with the state backend + // request. Its value is arbitrary and ignored by meshfed-api: the run JWT rides in + // the Basic password, and meshfed-api's TfStateRunTokenBasicAuthFilter reads only the + // password and interprets it as the token, rewriting Basic(:) to + // Bearer for the state endpoint. We send a placeholder "x" to make this explicit. + MeshStackRunTokenBasicUser = "x" ) type TfCmdParams struct { @@ -301,11 +308,12 @@ func (tfcmd *GenericTfCmd) createMeshStackHttpBackendFile() error { // long-lived credentials into a .tf file on disk. return fmt.Errorf("cannot configure meshStack HTTP backend: runToken is empty — the run object did not include an ephemeral token") } - // Use the run-scoped ephemeral token as a Bearer authorization header. - // This avoids embedding long-lived credentials in the generated backend config. - backendBlockBody.SetAttributeValue("headers", cty.ObjectVal(map[string]cty.Value{ - "Authorization": cty.StringVal("Bearer " + tfcmd.runContextInfo.runToken), - })) + // Auth is intentionally NOT baked into this backend config (no `headers` block): + // OpenTofu embeds the backend config verbatim into any saved plan, and a preflight + // APPLY may apply a plan produced by an earlier DETECT run days/weeks after that + // run's ephemeral key was revoked. Instead, the run token is supplied via + // TF_HTTP_USERNAME/TF_HTTP_PASSWORD (see buildTfEnv), which OpenTofu re-reads fresh + // at every plan/apply — OpenTofu's documented pattern for ephemeral backend credentials. tfcmd.Println(fmt.Sprintf("Writing backend config file: '%s'.", backendFileName)) return workingDirRoot.WriteFile(backendFileName, f.Bytes(), 0600) @@ -470,6 +478,20 @@ func (tfcmd *GenericTfCmd) buildTfEnv() (map[string]string, error) { } } + // MeshStackRunTokenBasicUser is the Basic-auth username sent with the state backend + // request. Its value is arbitrary and ignored by meshfed-api: the run JWT rides in + // the Basic password, and meshfed-api's TfStateRunTokenBasicAuthFilter reads only the + // password and interprets it as the token, rewriting Basic(:) to + // Bearer for the state endpoint. We send a placeholder "x" to make this explicit. + // + // Currently OpenTofu only supports setting basic auth for the backend, but in the future they may also support + // setting bearer auth directly: https://github.com/opentofu/opentofu/issues/2659 + if tfcmd.runContextInfo.useMeshBackendFallback && tfcmd.runContextInfo.runToken != "" { + env["TF_HTTP_USERNAME"] = MeshStackRunTokenBasicUser + env["TF_HTTP_PASSWORD"] = tfcmd.runContextInfo.runToken + envKeys = append(envKeys, "TF_HTTP_USERNAME") // do NOT log the password value + } + if len(envKeys) > 0 { msg := "Set the following env variables: " msg += strings.Join(envKeys, ", ") diff --git a/tf-block-runner/tfrun/tfcmd_test.go b/tf-block-runner/tfrun/tfcmd_test.go index 7a2786d..82c5c72 100644 --- a/tf-block-runner/tfrun/tfcmd_test.go +++ b/tf-block-runner/tfrun/tfcmd_test.go @@ -496,6 +496,46 @@ func Test_setEnvWith_DoesNotOverwriteExistingMeshStackRunVarsFile(t *testing.T) assert.Equal(t, string(customMeshStackRunVarsTf), string(content), "Existing file should not be overwritten") } +// Test_buildTfEnv_WithMeshBackendFallbackAndRunToken_SetsTfHttpBasicAuthEnv verifies that when the +// meshStack HTTP backend is in play, buildTfEnv supplies the run token via TF_HTTP_USERNAME/PASSWORD +// (Basic auth) rather than baking it into the backend config, so OpenTofu re-reads a live token at +// every plan/apply instead of embedding a since-revoked one into a saved plan. +func Test_buildTfEnv_WithMeshBackendFallbackAndRunToken_SetsTfHttpBasicAuthEnv(t *testing.T) { + uut := makeTestGenericTfCmd(t) + uut.runContextInfo.useMeshBackendFallback = true + uut.runContextInfo.runToken = "ephemeral-run-token" + + capturedEnv, err := uut.buildTfEnv() + require.NoError(t, err) + + assert.Equal(t, MeshStackRunTokenBasicUser, capturedEnv["TF_HTTP_USERNAME"]) + assert.Equal(t, "ephemeral-run-token", capturedEnv["TF_HTTP_PASSWORD"]) +} + +// Test_buildTfEnv_WithoutMeshBackendFallback_DoesNotSetTfHttpBasicAuthEnv verifies that +// buildTfEnv leaves TF_HTTP_* unset when the meshStack backend is not in play (e.g. a building +// block brings its own backend), and also when no runToken is available. +func Test_buildTfEnv_WithoutMeshBackendFallback_DoesNotSetTfHttpBasicAuthEnv(t *testing.T) { + uut := makeTestGenericTfCmd(t) + uut.runContextInfo.useMeshBackendFallback = false + uut.runContextInfo.runToken = "ephemeral-run-token" + + capturedEnv, err := uut.buildTfEnv() + require.NoError(t, err) + + assert.NotContains(t, capturedEnv, "TF_HTTP_USERNAME") + assert.NotContains(t, capturedEnv, "TF_HTTP_PASSWORD") + + uut.runContextInfo.useMeshBackendFallback = true + uut.runContextInfo.runToken = "" + + capturedEnv, err = uut.buildTfEnv() + require.NoError(t, err) + + assert.NotContains(t, capturedEnv, "TF_HTTP_USERNAME") + assert.NotContains(t, capturedEnv, "TF_HTTP_PASSWORD") +} + func Test_createMeshStackHttpBackendFile_MissingRunToken_ReturnsError(t *testing.T) { originalConfig := AppConfig AppConfig = TfRunnerConfig{ @@ -527,9 +567,10 @@ func Test_createMeshStackHttpBackendFile_MissingRunToken_ReturnsError(t *testing assert.Empty(t, filesWritten, "no backend file should be written when runToken is missing") } -// Test_createMeshStackHttpBackendFile_WithRunToken verifies that when a runToken is available -// (Kubernetes / single-run mode), the generated backend config uses a Bearer authorization -// header instead of static username/password credentials. +// Test_createMeshStackHttpBackendFile_WithRunToken verifies that the generated backend config +// contains only the state endpoint address and no auth (no headers/Authorization/Bearer) — the +// runToken is supplied to Terraform via TF_HTTP_USERNAME/TF_HTTP_PASSWORD env vars instead (see +// buildTfEnv), so nothing secret is baked into a saved plan. // It also verifies that meshstackBaseUrl from the run links is used as the backend base URL. func Test_createMeshStackHttpBackendFile_WithRunToken(t *testing.T) { originalConfig := AppConfig @@ -560,6 +601,10 @@ func Test_createMeshStackHttpBackendFile_WithRunToken(t *testing.T) { } asserted = true g.Assert(t, "backend", content) + assert.Contains(t, string(content), "address") + assert.NotContains(t, string(content), "headers") + assert.NotContains(t, string(content), "Authorization") + assert.NotContains(t, string(content), "Bearer") } return nil })) From 100cd8afd03301c16e7cbb097b140ceb9fb0a372 Mon Sep 17 00:00:00 2001 From: Stefan Tomm Date: Thu, 2 Jul 2026 09:03:24 +0200 Subject: [PATCH 2/3] fix: remove run related properties from auto.tfvars for plan and apply with artifact runs A saved plan embeds the exact variable values it was created with, but terraform still re-reads any *.auto.tfvars on disk when applying a saved plan and rejects the apply with "Mismatch between input and plan variable value" whenever a value differs. This runner regenerates its meshStack auto.tfvars with run-specific values (e.g. meshstack_building_block_run_id) on every run, which necessarily differ between the DETECT run that produced the plan and the APPLY run consuming it, so they are now removed from the auto tfvars in that case Also add PrintlnUser/PrintflnUser to surface which apply path was taken (predecessor plan vs. fresh apply) in the SystemMessage the user sees in meshStack, not just the local runner logs. --- tf-block-runner/tfrun/tfapply.go | 19 ++- tf-block-runner/tfrun/tfcmd.go | 68 ++++++++--- tf-block-runner/tfrun/tfcmd_test.go | 115 ++++++++++++++++-- tf-block-runner/tfrun/worker_scenario_test.go | 9 +- 4 files changed, 168 insertions(+), 43 deletions(-) diff --git a/tf-block-runner/tfrun/tfapply.go b/tf-block-runner/tfrun/tfapply.go index e135718..f078a90 100644 --- a/tf-block-runner/tfrun/tfapply.go +++ b/tf-block-runner/tfrun/tfapply.go @@ -166,21 +166,18 @@ func (tfcmd *TfApplyCommand) execute() { tfcmd.advanceStep(preRunUserMsg) - if tfcmd.params.planArtifactUrl != "" { - // This APPLY is linked to a predecessor DETECT run: instead of re-planning we apply the - // EXACT terraform plan that was previewed during that dry-run. The bytes are downloaded - // from the runner-facing planArtifact link with the run's bearer auth. - // - // terraform note: applying a saved plan rejects variable INPUT supplied via the -var / - // -var-file command-line flags, but auto-loaded *.auto.tfvars files present in the working - // directory are simply ignored (the saved plan embeds the variable values). Since this - // runner supplies variables exclusively via *.auto.tfvars files (see vars()), the existing - // vars()/saveInputFiles() steps stay in place and do not conflict with the saved plan. + if applyingPredecessorPlan := tfcmd.params.planArtifactUrl != ""; applyingPredecessorPlan { + // This APPLY is linked to a predecessor DETECT run, so we replay the exact plan previewed + // during that dry-run instead of computing a fresh one — the change is applied precisely as + // it was reviewed and approved. if err = tfcmd.applyPredecessorPlan(tf); err != nil { tfcmd.fail(err) return } } else { + // No predecessor plan artifact was linked to this APPLY run, so we run a fresh + // terraform apply. Surfaced to the user so the two apply paths are distinguishable. + tfcmd.PrintlnToLogsAndStep("No plan artifact linked to this run; running a fresh terraform apply.") // Variables are now in meshstack.auto.tfvars file, no command-line args needed if err = tf.Apply(tfcmd.ctx); err != nil { tfcmd.fail(err) @@ -217,7 +214,7 @@ func (tfcmd *TfApplyCommand) execute() { // telling the user to re-run the dry-run. Committing .terraform.lock.hcl in the building block does // not by itself avoid this, since init still runs with -upgrade. func (tfcmd *TfApplyCommand) applyPredecessorPlan(tf TfFacade) error { - tfcmd.Println("Applying predecessor plan artifact instead of re-planning.") + tfcmd.PrintlnToLogsAndStep("Using the provided plan artifact from the dry-run; applying it verbatim instead of re-planning.") planFile := tfcmd.runContextInfo.artifactFilePath f, err := os.OpenFile(planFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) diff --git a/tf-block-runner/tfrun/tfcmd.go b/tf-block-runner/tfrun/tfcmd.go index 7b61724..16917cd 100644 --- a/tf-block-runner/tfrun/tfcmd.go +++ b/tf-block-runner/tfrun/tfcmd.go @@ -75,6 +75,13 @@ func (tfcmd *GenericTfCmd) Printfln(format string, v ...any) { tfcmd.runContextInfo.logwrap.PrintlnToLocalLogs(fmt.Sprintf(format, v...)) } +// PrintlnToLogsAndStep writes to both the local operator logs and the update logs, so the +// message becomes part of the current step's SystemMessage that the end user sees +// in meshStack (in addition to the runner's local console logs). +func (tfcmd *GenericTfCmd) PrintlnToLogsAndStep(v ...any) { + tfcmd.runContextInfo.logwrap.PrintlnToLocalAndUpdateLogs(v...) +} + func (tfcmd *GenericTfCmd) fail(err error) { tfcmd.failWithUserMsg(err, nil) } @@ -489,7 +496,8 @@ func (tfcmd *GenericTfCmd) buildTfEnv() (map[string]string, error) { if tfcmd.runContextInfo.useMeshBackendFallback && tfcmd.runContextInfo.runToken != "" { env["TF_HTTP_USERNAME"] = MeshStackRunTokenBasicUser env["TF_HTTP_PASSWORD"] = tfcmd.runContextInfo.runToken - envKeys = append(envKeys, "TF_HTTP_USERNAME") // do NOT log the password value + // Only the key names are logged below, never the values, so listing both is safe and more debuggable. + envKeys = append(envKeys, "TF_HTTP_USERNAME", "TF_HTTP_PASSWORD") } if len(envKeys) > 0 { @@ -622,31 +630,51 @@ func (tfcmd *GenericTfCmd) vars() error { } // Add meshStack-provided variables to the tfvars file. - // Variable declarations are still provided via meshStack_run_vars.tf to prevent - // Terraform from complaining about undeclared variables. - // These variables should always be present. - meshStackVars := map[string]string{ - "meshstack_building_block_run_b64": tfcmd.runContextInfo.runJsonBase64, - "meshstack_building_block_id": tfcmd.runContextInfo.bbId, - "meshstack_building_block_run_id": tfcmd.runContextInfo.runId, + // + // Run-scoped vars (run_id, run_b64) are DEPRECATED and their value is omitted on a DETECT plan + // and on an APPLY replaying a predecessor's saved plan: applying a saved plan requires input + // values identical to plan time, but these necessarily differ between the DETECT run and the + // APPLY consuming its plan, so terraform would reject it with "Mismatch between input and plan + // variable value". They are always declared but optional/nullable so omitting the value is legal + // (reading as null in those modes); building blocks needing the full payload should read it from + // the pre-run script's stdin (see RunScript). building_block_id is stable across runs, so it is + // always written and stays required. + meshStackVars := []struct { + Name string + Value string + RunScoped bool // deprecated + optional; value only written on fresh apply/destroy + }{ + {Name: "meshstack_building_block_id", Value: tfcmd.runContextInfo.bbId}, + {Name: "meshstack_building_block_run_b64", Value: tfcmd.runContextInfo.runJsonBase64, RunScoped: true}, + {Name: "meshstack_building_block_run_id", Value: tfcmd.runContextInfo.runId, RunScoped: true}, } meshStackVarsFile := hclwrite.NewEmptyFile() - for varName, varValue := range util.SortedByKeys(meshStackVars) { - diags = varsFile.AddVariable(varName, varValue, AddVariableOptions{}) - if diags.HasErrors() { - for _, diag := range diags { - _, _ = tfcmd.runContextInfo.logwrap.PrintlnToUpdateLogs(fmt.Sprintf( - "While adding variable '%s': %s: %s", varName, diag.Summary, diag.Detail, - )) + for _, variable := range meshStackVars { + includeRunScopedVars := tfcmd.params.runMode != DETECT.str() && tfcmd.params.planArtifactUrl == "" + if !variable.RunScoped || includeRunScopedVars { + diags = varsFile.AddVariable(variable.Name, variable.Value, AddVariableOptions{}) + if diags.HasErrors() { + for _, diag := range diags { + _, _ = tfcmd.runContextInfo.logwrap.PrintlnToUpdateLogs(fmt.Sprintf( + "While adding variable '%s': %s: %s", variable.Name, diag.Summary, diag.Detail, + )) + } + continue } + } + // Declare the variable unless the building block already declares it itself. + if _, variableBlockExists := existingVariableInputs[variable.Name]; variableBlockExists { + tfcmd.Println(fmt.Sprintf("Skip defining variable block for %s as it is already present in configuration", variable.Name)) continue } - if _, variableBlockExists := existingVariableInputs[varName]; !variableBlockExists { - variableBlockBody := meshStackVarsFile.Body().AppendNewBlock("variable", []string{varName}).Body() - variableBlockBody.SetAttributeTraversal("type", hcl.Traversal{hcl.TraverseRoot{Name: "string"}}) - variableBlockBody.SetAttributeValue("nullable", cty.BoolVal(false)) + variableBlockBody := meshStackVarsFile.Body().AppendNewBlock("variable", []string{variable.Name}).Body() + variableBlockBody.SetAttributeTraversal("type", hcl.Traversal{hcl.TraverseRoot{Name: "string"}}) + if variable.RunScoped { + // Deprecated run-scoped variable: optional so it can be omitted on dry-runs and saved-plan replays. + variableBlockBody.SetAttributeValue("nullable", cty.BoolVal(true)) + variableBlockBody.SetAttributeValue("default", cty.NullVal(cty.String)) } else { - tfcmd.Println(fmt.Sprintf("Skip defining variable block for %s as it is already present in configuration", varName)) + variableBlockBody.SetAttributeValue("nullable", cty.BoolVal(false)) } } diff --git a/tf-block-runner/tfrun/tfcmd_test.go b/tf-block-runner/tfrun/tfcmd_test.go index 82c5c72..87e2a5a 100644 --- a/tf-block-runner/tfrun/tfcmd_test.go +++ b/tf-block-runner/tfrun/tfcmd_test.go @@ -415,10 +415,13 @@ func Test_extractContentFromDataUrl(t *testing.T) { } } -// Test_prepareEnv_CreatesMeshStackRunVarsFile tests that: -// 1. The meshStack_run_vars.tf file is created when it doesn't exist -// Note: meshStack variables are now in *.auto.tfvars, not environment variables -// Only variables not already defined will be written! +// Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv verifies that: +// 1. The meshStack_run_vars.tf file is created when it doesn't exist. +// 2. All three meshStack variables are declared: the building block id as required, and the +// deprecated run-scoped variables (run id / run b64) as optional (nullable, default null) so +// they can be omitted on dry-runs and saved-plan replays — see vars(). +// +// Note: meshStack variables are provided via *.auto.tfvars, not environment variables. func Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv(t *testing.T) { uut := makeTestGenericTfCmd(t) @@ -427,10 +430,6 @@ func Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv(t *testing.T) { uut.runContextInfo.bbId = "test-bb-id" uut.runContextInfo.runId = "test-run-id" - // Create an existing variables.tf file with one variable defined - err := os.WriteFile(path.Join(uut.runContextInfo.workingDirectory, "variables.tf"), customMeshStackRunVarsTf, 0644) - require.NoError(t, err) - // Call buildTfEnv capturedEnv, err := uut.buildTfEnv() require.NoError(t, err) @@ -439,14 +438,17 @@ func Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv(t *testing.T) { err = uut.vars() require.NoError(t, err) - // Verify the file contains the expected variable declarations + // Verify the file declares the building block id (required) and the deprecated run-scoped + // variables as optional (nullable, default null). meshStackVarsPath := path.Join(uut.runContextInfo.workingDirectory, "meshStack_run_vars.tf") content, err := os.ReadFile(meshStackVarsPath) require.NoError(t, err) contentStr := string(content) + assert.Contains(t, contentStr, "variable \"meshstack_building_block_id\"") assert.Contains(t, contentStr, "variable \"meshstack_building_block_run_b64\"") - assert.NotContains(t, contentStr, "variable \"meshstack_building_block_id\"") // defined in custom variables.tf assert.Contains(t, contentStr, "variable \"meshstack_building_block_run_id\"") + assert.Contains(t, contentStr, "default") + assert.Contains(t, contentStr, "null") // Verify the meshStack variables are NOT in environment (they go to *.auto.tfvars instead) require.NotNil(t, capturedEnv, "Environment should be set") @@ -455,6 +457,99 @@ func Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv(t *testing.T) { assert.NotContains(t, capturedEnv, "TF_VAR_meshstack_building_block_run_id", "meshStack variables should not be in env") } +func readGeneratedTfvars(t *testing.T, uut *GenericTfCmd) string { + t.Helper() + tfvarsPath := path.Join(uut.runContextInfo.workingDirectory, "aaaaaa_meshstack-e48f8924-a6c0-4ff0-9528-ff3c1f6f94d8.auto.tfvars") + content, err := os.ReadFile(tfvarsPath) + require.NoError(t, err) + return string(content) +} + +// Test_vars_OmitsRunScopedVarValuesOnDetectAndSavedPlanReplay verifies the saved-plan invariant: +// the deprecated run-scoped variables (run id / run b64) get a value written into auto.tfvars only +// for a fresh apply/destroy. On a DETECT plan (so nothing run-scoped is baked into the plan) and on +// an APPLY replaying a predecessor plan (planArtifactUrl set, so no "Mismatch between input and plan +// variable value") their value is omitted. The building block id is always written, and all three +// remain declared regardless so building blocks referencing them still parse. +func Test_vars_OmitsRunScopedVarValuesOnDetectAndSavedPlanReplay(t *testing.T) { + cases := []struct { + name string + runMode string + planArtifactUrl string + wantRunScoped bool + }{ + {name: "fresh apply writes run-scoped values", runMode: "APPLY", wantRunScoped: true}, + {name: "destroy writes run-scoped values", runMode: "DESTROY", wantRunScoped: true}, + {name: "detect omits run-scoped values", runMode: "DETECT", wantRunScoped: false}, + {name: "apply replaying predecessor plan omits run-scoped values", runMode: "APPLY", planArtifactUrl: "https://example/artifact", wantRunScoped: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + uut := makeTestGenericTfCmd(t) + uut.params.runMode = tc.runMode + uut.params.planArtifactUrl = tc.planArtifactUrl + uut.runContextInfo.bbId = "some-bbd-id-123" + uut.runContextInfo.runId = "some-run-id-12345" + uut.runContextInfo.runJsonBase64 = "c29tZS1ydW4tanNvbg==" + + require.NoError(t, uut.vars()) + + tfvars := readGeneratedTfvars(t, uut) + // The building block id is stable across runs and always written. + assert.Regexp(t, `meshstack_building_block_id\s+= "some-bbd-id-123"`, tfvars) + + if tc.wantRunScoped { + assert.Regexp(t, `meshstack_building_block_run_id\s+= "some-run-id-12345"`, tfvars) + assert.Regexp(t, `meshstack_building_block_run_b64\s+= "c29tZS1ydW4tanNvbg=="`, tfvars) + } else { + assert.NotContains(t, tfvars, "meshstack_building_block_run_id") + assert.NotContains(t, tfvars, "meshstack_building_block_run_b64") + } + + // Regardless of run mode, all three variables must be declared so modules that reference + // them keep parsing. The run-scoped ones are optional (default null). + declPath := path.Join(uut.runContextInfo.workingDirectory, "meshStack_run_vars.tf") + decl, err := os.ReadFile(declPath) + require.NoError(t, err) + assert.Contains(t, string(decl), `variable "meshstack_building_block_id"`) + assert.Contains(t, string(decl), `variable "meshstack_building_block_run_id"`) + assert.Contains(t, string(decl), `variable "meshstack_building_block_run_b64"`) + }) + } +} + +// Test_vars_SkipsDeclaringVariablesAlreadyDeclaredByBuildingBlock verifies that when a building +// block already declares a meshStack variable in its own configuration, the runner does NOT emit a +// duplicate declaration in the generated meshStack_run_vars.tf (a duplicate would make terraform +// fail with "Duplicate variable declaration"). The variable's value is still written to auto.tfvars. +// This restores coverage of the existingVariableInputs skip branch that the pre-existing variables.tf +// version of Test_setEnvWith_CreatesMeshStackRunVarsFileAndSetsTfVarEnv used to exercise. +func Test_vars_SkipsDeclaringVariablesAlreadyDeclaredByBuildingBlock(t *testing.T) { + uut := makeTestGenericTfCmd(t) + uut.runContextInfo.bbId = "test-bb-id" + uut.runContextInfo.runId = "test-run-id" + uut.runContextInfo.runJsonBase64 = "dGVzdC1ydW4tanNvbi1iYXNlNjQ=" + + // The building block declares meshstack_building_block_id itself (see the embedded fixture). + err := os.WriteFile(path.Join(uut.runContextInfo.workingDirectory, "variables.tf"), customMeshStackRunVarsTf, 0644) + require.NoError(t, err) + + require.NoError(t, uut.vars()) + + // The runner must not re-declare the already-declared variable, but must still declare the others. + declPath := path.Join(uut.runContextInfo.workingDirectory, "meshStack_run_vars.tf") + decl, err := os.ReadFile(declPath) + require.NoError(t, err) + declStr := string(decl) + assert.NotContains(t, declStr, `variable "meshstack_building_block_id"`, "must not duplicate a building-block-declared variable") + assert.Contains(t, declStr, `variable "meshstack_building_block_run_id"`) + assert.Contains(t, declStr, `variable "meshstack_building_block_run_b64"`) + + // The value is still provided via auto.tfvars regardless of who declares the variable. + assert.Regexp(t, `meshstack_building_block_id\s+= "test-bb-id"`, readGeneratedTfvars(t, uut)) +} + // Test_setEnvWith_DoesNotOverwriteExistingMeshStackRunVarsFile tests that // if the meshStack_run_vars.tf file already exists, it is not overwritten func Test_setEnvWith_DoesNotOverwriteExistingMeshStackRunVarsFile(t *testing.T) { diff --git a/tf-block-runner/tfrun/worker_scenario_test.go b/tf-block-runner/tfrun/worker_scenario_test.go index 7ba2ed8..4515756 100644 --- a/tf-block-runner/tfrun/worker_scenario_test.go +++ b/tf-block-runner/tfrun/worker_scenario_test.go @@ -405,7 +405,7 @@ func (suite *WorkerTestSuite) Test_ApplyTfFailure() { // UserMessage is now set to the error text for better panel visibility assert.NotNil(suite.T(), executeTf.UserMessage) assert.Equal(suite.T(), "test error", *executeTf.UserMessage) - assert.Equal(suite.T(), "apply in progress\nfailure\ntest error\n", *executeTf.SystemMessage) // includes error message + assert.Equal(suite.T(), "No plan artifact linked to this run; running a fresh terraform apply.\napply in progress\nfailure\ntest error\n", *executeTf.SystemMessage) // includes error message outputStep := findStep(suite.T(), update, StepOutput) assert.Equal(suite.T(), FAILED.str(), *outputStep.Status) @@ -526,8 +526,13 @@ func (suite *WorkerTestSuite) Test_UpdatesStatusWithLiveLogs() { assert.GreaterOrEqual(suite.T(), len(updateCalls), 9) // updates every 500ms, apply duration is minimum 5s // to verify, we check if there are at least the expected updates existing - // with the systemMessages ["0", "01", "012", "0123", "01234"] in the execute_tf step + // with the systemMessages ["0", "01", "012", "0123", "01234"] in the execute_tf step, + // prefixed with the "no predecessor plan" notice logged before a fresh apply. + const noPredecessorNotice = "No plan artifact linked to this run; running a fresh terraform apply.\n" expectedLogUpdates := []string{"0", "01", "012", "0123", "01234"} + for i, suffix := range expectedLogUpdates { + expectedLogUpdates[i] = noPredecessorNotice + suffix + } for _, expected := range expectedLogUpdates { found := false From 442876fbc6a4ac146bec182e2f708b3a37b35c1c Mon Sep 17 00:00:00 2001 From: Stefan Tomm Date: Thu, 2 Jul 2026 12:03:11 +0200 Subject: [PATCH 3/3] fix: revert the artifact url same originc heck this did not work locally as we use the mux proxy that runs on a different port than what is provided as localhost urls from teh hal link. In the end, it was a very safe guard solution before, but I don't see it being relevant for our application. --- go-meshapi-client/meshapi/client.go | 28 ------------------------ go-meshapi-client/meshapi/client_test.go | 21 ------------------ 2 files changed, 49 deletions(-) diff --git a/go-meshapi-client/meshapi/client.go b/go-meshapi-client/meshapi/client.go index c1abdab..9fce7af 100644 --- a/go-meshapi-client/meshapi/client.go +++ b/go-meshapi-client/meshapi/client.go @@ -6,8 +6,6 @@ import ( "fmt" "io" "net/http" - "net/url" - "strings" ) const ( @@ -127,14 +125,6 @@ func (c *Client) FetchRun(runnerUUID string) (*RunDetailsDTO, []byte, error) { // buffering it, since artifacts like terraform plans can be large. A non-2xx response is returned // as an error rather than treated as empty, so a missing/expired artifact fails the run visibly. func (c *Client) DownloadArtifact(artifactURL string, w io.Writer) error { - // Enforce same-origin (scheme + host) as the configured baseURL before attaching auth. The - // artifact URL is an absolute href taken from an API response; if a malicious or buggy API ever - // returned an href pointing at a different host, setHeaders would leak the run bearer token to - // that host (and enable SSRF-style requests through the runner). Reject any cross-origin URL. - if err := c.assertSameOrigin(artifactURL); err != nil { - return err - } - req, err := http.NewRequest(http.MethodGet, artifactURL, nil) if err != nil { return fmt.Errorf("download artifact %s: failed to create request: %w", artifactURL, err) @@ -171,24 +161,6 @@ func (c *Client) DownloadArtifact(artifactURL string, w io.Writer) error { return nil } -// This guards DownloadArtifact against leaking the run bearer -// token — attached by setHeaders — to any host other than the meshfed API the client was created for. -func (c *Client) assertSameOrigin(artifactURL string) error { - base, err := url.Parse(c.baseURL) - if err != nil { - return fmt.Errorf("download artifact %s: invalid client baseURL %q: %w", artifactURL, c.baseURL, err) - } - target, err := url.Parse(artifactURL) - if err != nil { - return fmt.Errorf("download artifact %s: invalid artifact URL: %w", artifactURL, err) - } - if !strings.EqualFold(target.Scheme, base.Scheme) || !strings.EqualFold(target.Host, base.Host) { - return fmt.Errorf("download artifact %s: refusing to send authenticated request to a host other than %s://%s", - artifactURL, base.Scheme, base.Host) - } - return nil -} - // RegisterSource registers the caller as a status source for the given run via POST. // If the source is already registered (HTTP 409 Conflict) the call is treated as a no-op. func (c *Client) RegisterSource(runID string, registration RegistrationDTO) error { diff --git a/go-meshapi-client/meshapi/client_test.go b/go-meshapi-client/meshapi/client_test.go index 0279ed8..cab36a9 100644 --- a/go-meshapi-client/meshapi/client_test.go +++ b/go-meshapi-client/meshapi/client_test.go @@ -38,27 +38,6 @@ func TestDownloadArtifact_StreamsBodyIntoWriter(t *testing.T) { assert.Equal(t, "Bearer run-token", gotAuth, "run auth should still be applied") } -func TestDownloadArtifact_RejectsCrossOriginURL(t *testing.T) { - // The artifact host must match the client's configured baseURL, otherwise the run bearer token - // attached by setHeaders would leak to a foreign host. The request must never be issued. - var reached bool - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - reached = true - w.WriteHeader(http.StatusOK) - })) - defer srv.Close() - - client := NewClientWithHTTP("https://api.example.com", "test-node", BearerTokenAuth{Token: "run-token"}, srv.Client()) - - var buf bytes.Buffer - err := client.DownloadArtifact(srv.URL+"/artifact", &buf) - - require.Error(t, err) - assert.Contains(t, err.Error(), "refusing to send authenticated request") - assert.False(t, reached, "the cross-origin request must not be issued") - assert.Empty(t, buf.Bytes()) -} - func TestDownloadArtifact_Non2xxReturnsErrorWithBody(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "artifact expired", http.StatusNotFound)