Feature/fix tf runs with artifacts

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 {

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)

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"
-    }
   }
 }

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)

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(<any-user>:<jwt>) to
+	// Bearer <jwt> for the state endpoint. We send a placeholder "x" to make this explicit.
+	MeshStackRunTokenBasicUser = "x"
 )
 
 type TfCmdParams struct {
@@ -68,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)
 }
@@ -301,11 +315,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 +485,21 @@ 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(<any-user>:<jwt>) to
+	// Bearer <jwt> 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
+		// 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 {
 		msg := "Set the following env variables: "
 		msg += strings.Join(envKeys, ", ")
@@ -600,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))
 		}
 	}