fix: neutralize untrusted workspace strings and redact credential secrets

CHANGELOG.md

@@ -11,6 +11,11 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Security
 
+- Progress lines, the `auth status` table, and generated HCL comments neutralize control,
+  format, and line separator characters that come from workspace object names.
+- `inventory.json` and `inventory --json` no longer serialize storage credential secret fields.
+- Grant pagination stops with an issue after 1000 pages or a repeated page token.
+- Telemetry records `workspace_error` and `render_error` instead of collapsing them to `other`.
 - The release job no longer receives the winget token or runs tests with release credentials.
 - Release builds use the commit timestamp for the build date and module timestamps.
 - The installers require HTTPS and TLS 1.2, and match checksum entries exactly.

docs/telemetry.md

@@ -96,7 +96,8 @@ Every event has these fields and no others:
 Error categories are `none`, `other`, `invalid_usage`, `configuration_error`,
 `authentication_failed`, `permission_denied`, `not_found`, `resource_exhausted`,
 `service_error`, `api_error`, `connection_failed`, `operation_failed`, `timeout`,
-`canceled`, `partial_result`, and `output_error`. Unknown categories become `other`.
+`canceled`, `partial_result`, `output_error`, `workspace_error`, and `render_error`.
+Unknown categories become `other`.
 
 `--allow-partial` still reports `partial`, even when the command exits successfully.
 Default resource groups also count as selected types.

internal/cli/auth.go

@@ -45,11 +45,11 @@ Use inventory --json to check the visible resource selection.`,
 			}
 			rc.out.Success("authenticated")
 			rc.out.Table([]string{"KEY", "VALUE"}, [][]string{
-				{"host", id.Host},
-				{"user", id.UserName},
-				{"auth_type", id.AuthType},
+				{"host", terminalText(id.Host)},
+				{"user", terminalText(id.UserName)},
+				{"auth_type", terminalText(id.AuthType)},
 				{"workspace_id", fmt.Sprint(id.WorkspaceID)},
-				{"metastore_id", id.MetastoreID},
+				{"metastore_id", terminalText(id.MetastoreID)},
 			})
 			return nil
 		},

internal/cli/errors.go

@@ -86,10 +86,12 @@ func outputError(err error) error {
 		"Check the destination and disk access. Use --out with a new directory. Keep existing files.")
 }
 
+// terminalText escapes control, format, and line separator characters so untrusted
+// workspace strings cannot rewrite, hide, or reorder terminal output.
 func terminalText(value string) string {
 	var text strings.Builder
 	for _, char := range value {
-		if unicode.IsControl(char) {
+		if unicode.IsControl(char) || unicode.Is(unicode.Cf, char) || char == '\u2028' || char == '\u2029' {
 			quoted := strconv.QuoteRune(char)
 			text.WriteString(quoted[1 : len(quoted)-1])
 		} else {

internal/cli/errors_test.go

@@ -193,3 +193,29 @@ func TestOutputFailureDiagnostic(t *testing.T) {
 		t.Fatalf("unexpected diagnostic: %v", failure)
 	}
 }
+
+func TestTerminalTextNeutralizesControlAndFormatCharacters(t *testing.T) {
+	for _, hostile := range []string{
+		"a\x1b[2Kb", "a\rb", "a\u202eb", "a\u2066b", "a\u2028b", "a\u200fb",
+	} {
+		safe := terminalText(hostile)
+		if strings.ContainsAny(safe, "\x1b\r\u202e\u2066\u2028\u200f") {
+			t.Fatalf("%q was not neutralized: %q", hostile, safe)
+		}
+		if !strings.HasPrefix(safe, "a") || !strings.HasSuffix(safe, "b") {
+			t.Fatalf("%q lost visible text: %q", hostile, safe)
+		}
+	}
+	if terminalText("Analytics WH") != "Analytics WH" {
+		t.Fatal("plain text changed")
+	}
+}
+
+func TestProgressOutputNeutralizesWorkspaceNames(t *testing.T) {
+	var stderr strings.Builder
+	rc := &runtime{stderr: &stderr, g: &globals{}}
+	rc.progress()("  Warehouse: %s", "prod\x1b[2K\rok")
+	if got := stderr.String(); strings.Contains(got, "\x1b") || strings.Contains(got, "\r") {
+		t.Fatalf("progress leaked control characters: %q", got)
+	}
+}

internal/cli/root.go

@@ -217,7 +217,12 @@ func (rc *runtime) progress() func(string, ...any) {
 		return nil
 	}
 	return func(format string, args ...any) {
-		_, _ = fmt.Fprintf(rc.stderr, format+"\n", args...)
+		// Workspace object names are untrusted. Neutralize terminal control sequences at the sink.
+		safe := make([]any, len(args))
+		for i, arg := range args {
+			safe[i] = terminalText(fmt.Sprint(arg))
+		}
+		_, _ = fmt.Fprintf(rc.stderr, format+"\n", safe...)
 	}
 }
 

internal/cli/telemetry_codes_test.go

@@ -0,0 +1,36 @@
+package cli
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/536tech/datatf/internal/telemetry"
+)
+
+// Every error code the CLI attaches with withHint, plus every diagnostic category, must be
+// an allowlisted telemetry category. Otherwise the collector records it as "other".
+func TestTelemetryAllowsEveryCLIErrorCode(t *testing.T) {
+	for _, code := range []string{
+		"invalid_usage", "configuration_error", "authentication_failed", "permission_denied",
+		"not_found", "resource_exhausted", "service_error", "api_error", "connection_failed",
+		"operation_failed", "timeout", "canceled", "partial_result", "output_error",
+		"workspace_error", "render_error",
+	} {
+		payload, err := telemetry.Payload(telemetry.Run{
+			Version: "1.0.0", OS: "linux", Arch: "amd64", Command: "export", Scope: "workspace",
+			Outcome: "error", ErrorCode: code,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		var event struct {
+			ErrorCode string `json:"error_code"`
+		}
+		if err := json.Unmarshal(payload, &event); err != nil {
+			t.Fatal(err)
+		}
+		if event.ErrorCode != code {
+			t.Errorf("%s recorded as %s", code, event.ErrorCode)
+		}
+	}
+}

internal/contract/hcl.go

@@ -186,9 +186,13 @@ func appendTraversalIndex(tokens, index hclwrite.Tokens) hclwrite.Tokens {
 	return append(tokens, &hclwrite.Token{Type: hclsyntax.TokenCBrack, Bytes: []byte("]")})
 }
 
+// commentLine is the only text that bypasses hclwrite. A line break would end the comment,
+// so it is never allowed through.
+var commentLine = strings.NewReplacer("\r\n", " ", "\r", " ", "\n", " ", "\u2028", " ", "\u2029", " ")
+
 func writeComments(buf *bytes.Buffer, lines []string) {
 	for _, line := range lines {
-		buf.WriteString("# " + line + "\n")
+		buf.WriteString("# " + commentLine.Replace(line) + "\n")
 	}
 }
 

internal/inventory/redact.go

@@ -0,0 +1,20 @@
+package inventory
+
+import "github.com/databricks/databricks-sdk-go/service/catalog"
+
+// redactStorageCredential drops credential secret fields before the SDK struct is
+// serialized into inventory files or JSON output. Unity Catalog redacts these values on
+// read today; DataTF never relies on that.
+func redactStorageCredential(info catalog.StorageCredentialInfo) catalog.StorageCredentialInfo {
+	if info.AzureServicePrincipal != nil {
+		sp := *info.AzureServicePrincipal
+		sp.ClientSecret = ""
+		info.AzureServicePrincipal = &sp
+	}
+	if info.CloudflareApiToken != nil {
+		token := *info.CloudflareApiToken
+		token.SecretAccessKey = ""
+		info.CloudflareApiToken = &token
+	}
+	return info
+}

internal/inventory/redact_test.go

@@ -0,0 +1,42 @@
+package inventory
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/databricks/databricks-sdk-go/service/catalog"
+)
+
+func TestRedactStorageCredentialDropsSecrets(t *testing.T) {
+	info := catalog.StorageCredentialInfo{
+		Name: "adls",
+		AzureServicePrincipal: &catalog.AzureServicePrincipal{
+			ApplicationId: "app", DirectoryId: "dir", ClientSecret: "canary-secret",
+		},
+		CloudflareApiToken: &catalog.CloudflareApiToken{
+			AccessKeyId: "key", AccountId: "acct", SecretAccessKey: "canary-token",
+		},
+	}
+	original, err := json.Marshal(info)
+	if err != nil {
+		t.Fatal(err)
+	}
+	redacted, err := json.Marshal(redactStorageCredential(info))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, canary := range []string{"canary-secret", "canary-token"} {
+		if strings.Contains(string(redacted), canary) {
+			t.Fatalf("secret survived redaction: %s", redacted)
+		}
+	}
+	for _, kept := range []string{`"application_id":"app"`, `"directory_id":"dir"`, `"access_key_id":"key"`} {
+		if !strings.Contains(string(redacted), kept) {
+			t.Fatalf("non-secret field dropped: %s", redacted)
+		}
+	}
+	if !strings.Contains(string(original), "canary-secret") {
+		t.Fatal("redaction mutated the caller's value")
+	}
+}

internal/inventory/unitycatalog.go

@@ -68,11 +68,14 @@ func sortedWorkspaceBindings(bindings []catalog.WorkspaceBinding) []catalog.Work
 	return out
 }
 
+// maxGrantPages bounds a hostile or looping paginated response.
+const maxGrantPages = 1000
+
 // grants reads direct privilege assignments on a securable, following pages.
 func (r *Reader) grants(ctx context.Context, securableType, fullName string) ([]Grant, bool) {
 	out := []Grant{}
 	pageToken := ""
-	for {
+	for page := 1; ; page++ {
 		resp, err := r.ws.Grants.Get(ctx, catalog.GetGrantRequest{
 			SecurableType: securableType,
 			FullName:      fullName,
@@ -93,6 +96,11 @@ func (r *Reader) grants(ctx context.Context, securableType, fullName string) ([]
 		if resp.NextPageToken == "" {
 			break
 		}
+		if resp.NextPageToken == pageToken || page >= maxGrantPages {
+			r.issue("unity_catalog", fullName, securableType+" grants",
+				fmt.Errorf("pagination did not finish after %d pages", page))
+			return nil, false
+		}
 		pageToken = resp.NextPageToken
 	}
 	sort.SliceStable(out, func(i, j int) bool { return out[i].Principal < out[j].Principal })
@@ -241,7 +249,8 @@ func (r *Reader) readStorageCredentials(ctx context.Context) {
 			)
 			r.logf("  Storage credential: %s (%s)", name, ownership)
 			sc := &StorageCredential{
-				Info: *info, Ownership: ownership, WorkspaceIDs: ids, WorkspaceBindings: bindings,
+				Info: redactStorageCredential(*info), Ownership: ownership,
+				WorkspaceIDs: ids, WorkspaceBindings: bindings,
 			}
 			sc.Grants, sc.GrantsRead = r.grants(gctx, "storage_credential", name)
 			if sc.Grants == nil {

internal/telemetry/event.go

@@ -40,6 +40,7 @@ var codes = []string{
 	"none", "other", "invalid_usage", "configuration_error", "authentication_failed",
 	"permission_denied", "not_found", "resource_exhausted", "service_error", "api_error",
 	"connection_failed", "operation_failed", "timeout", "canceled", "partial_result", "output_error",
+	"workspace_error", "render_error",
 }
 
 var releaseVersion = regexp.MustCompile(