Merge pull request #25 from andreagrandi/pii-redaction-docs-tests

Strengthen PII redaction docs and regression tests

Author: andreagrandi

CHANGELOG.md

@@ -5,6 +5,8 @@
 - Add `MB_SESSION_TOKEN` as an alternative authentication method for users without admin access to mint an API key; mutually exclusive with `MB_API_KEY` (#9)
 - Document changelog update workflow in `AGENTS.md` and add a pull request template prompting contributors to update `CHANGELOG.md` for user-visible changes (#20)
 - Move main package to `cmd/mb-cli` so `go install github.com/andreagrandi/mb-cli/cmd/mb-cli@latest` produces an `mb-cli` binary that matches the documented command name (#10)
+- Expand PII redaction documentation with covered commands, semantic-type enrichment behavior, known gaps, and export opt-out instructions (#17)
+- Enrich semantic types on non-parameterized `card run` results so PII columns from native saved questions are redacted consistently with the parameterized path (#17)
 
 ## [0.2.0] - 2026-03-12
 

README.md

@@ -263,9 +263,57 @@ The dashboard and parameter workflows use these Metabase endpoints:
 
 ## PII Redaction
 
-When AI agents use mb-cli directly (via shell commands), query results containing PII (emails, names, phone numbers) flow through stdout into the model's context. This feature prevents agents from seeing sensitive data by redacting PII columns before data leaves the client layer. Agents can still cross-reference records using IDs; for actual PII values, the user can check directly in Metabase.
+When AI agents use mb-cli directly (via shell commands), query results containing PII (emails, names, phone numbers) flow through stdout into the model's context. mb-cli prevents that by replacing PII column values with `[REDACTED]` before data leaves the client layer. Agents can still cross-reference records using IDs; for actual PII values, the user can check directly in Metabase.
 
-The approach leverages Metabase's own field semantic types (type/Email, type/Name, etc.). Redaction is ON by default — no configuration needed. Users must explicitly opt out with `MB_REDACT_PII=false` or `--redact-pii=false`. This is defense-in-depth: it makes the default path safe. An agent would have to actively choose to bypass it (a visible, auditable action).
+Redaction is **ON by default**. Disabling it is a visible, auditable action: when the flag or env var is set to `false`, mb-cli prints `Warning: PII redaction is disabled` to stderr on every invocation.
+
+### What gets redacted
+
+Columns whose Metabase field semantic type is one of:
+
+`type/Email`, `type/Name`, `type/Phone`, `type/Address`, `type/City`, `type/State`, `type/ZipCode`, `type/Country`, `type/Latitude`, `type/Longitude`, `type/Birthdate`, `type/AvatarURL`, `type/URL`, `type/ImageURL`, `type/Company`.
+
+For native SQL results, Metabase does not always populate the semantic type on result columns. mb-cli fills it in by fetching field metadata from the source database and matching by column name. When two fields share a name and disagree, mb-cli picks the PII type (err on the side of caution).
+
+### Where redaction applies
+
+| Command | Redacted? |
+|---------|-----------|
+| `query sql` | Yes (with column-name enrichment) |
+| `query filter` (MBQL) | Yes |
+| `card run` (with or without `--param`) | Yes (with column-name enrichment) |
+| `dashboard run-card` | Yes (with column-name enrichment) |
+| `table data` | Yes |
+| `field values` | Yes, when the field's semantic type is PII |
+
+Schema and metadata commands (`database metadata`, `table metadata`, `field get`, etc.) return field definitions, not data, so there is nothing to redact.
+
+### Limits and known gaps
+
+- Redaction relies on Metabase semantic types. A column holding PII without a semantic type (or with a custom type not in the list above) will not be redacted. Set semantic types in Metabase to close that gap.
+- Derived columns from joins, aggregations, or `CASE` expressions may lose the upstream semantic type. Native SQL enrichment matches by column name only — alias derived columns to match a known PII field name if you want them caught.
+- Free-form `type/Description` fields are not treated as PII; if you store names or emails in description-style columns, set their semantic type explicitly.
+- Redaction operates on result rows, not column headers. Column names like `email` remain visible — values are replaced.
+
+### Export behavior
+
+When redaction is enabled, the `--export` flag on `query sql` and `query filter` is **blocked** with this error:
+
+```
+export is not supported when PII redaction is enabled (use JSON or table format instead)
+```
+
+Raw export bytes (CSV/XLSX/JSON straight from Metabase) cannot be post-processed reliably for redaction, so the safe default is to refuse the operation. To export, explicitly opt out of redaction for that invocation:
+
+```bash
+MB_REDACT_PII=false mb-cli query sql --db 1 --sql "..." --export csv
+# or
+mb-cli query sql --redact-pii=false --db 1 --sql "..." --export csv
+```
+
+### Opt-out precedence
+
+The `--redact-pii` flag wins if set; otherwise `MB_REDACT_PII` is consulted; otherwise the default is `true`. Any value other than the literal string `false` is treated as enabled.
 
 ## License
 

internal/client/cards.go

@@ -41,27 +41,23 @@ func (c *Client) GetCard(id int) (*Card, error) {
 
 // RunCard executes a saved question and returns the query result.
 func (c *Client) RunCard(id int) (*QueryResult, error) {
-	resp, err := c.Post(fmt.Sprintf("/api/card/%d/query", id), nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to run card %d: %w", id, err)
-	}
-
-	return c.decodeCardQueryResult(resp, 0)
+	return c.RunCardWithParams(id, nil)
 }
 
-// RunCardWithParams executes a saved question with parameter values.
+// RunCardWithParams executes a saved question, optionally with parameter values.
+// The card is always fetched so semantic types can be enriched for redaction,
+// keeping behavior consistent between parameterized and non-parameterized runs.
 func (c *Client) RunCardWithParams(id int, params map[string]string) (*QueryResult, error) {
-	if len(params) == 0 {
-		return c.RunCard(id)
-	}
-
 	card, err := c.GetCard(id)
 	if err != nil {
 		return nil, err
 	}
 
-	body := map[string]any{
-		"parameters": buildCardQueryParameters(card, params),
+	var body any
+	if len(params) > 0 {
+		body = map[string]any{
+			"parameters": buildCardQueryParameters(card, params),
+		}
 	}
 
 	resp, err := c.Post(fmt.Sprintf("/api/card/%d/query", id), body)

tests/cards_test.go

@@ -94,24 +94,29 @@ func TestGetCard(t *testing.T) {
 
 func TestRunCard(t *testing.T) {
 	c, server := setupCardTestClient(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path != "/api/card/1/query" {
-			t.Errorf("expected path '/api/card/1/query', got %s", r.URL.Path)
-		}
-		if r.Method != "POST" {
-			t.Errorf("expected POST, got %s", r.Method)
-		}
-
 		w.Header().Set("Content-Type", "application/json")
-		json.NewEncoder(w).Encode(map[string]any{
-			"data": map[string]any{
-				"cols": []map[string]any{
-					{"name": "count", "display_name": "Count", "base_type": "type/Integer"},
-				},
-				"rows": [][]any{
-					{42},
+		switch r.URL.Path {
+		case "/api/card/1":
+			json.NewEncoder(w).Encode(map[string]any{
+				"id": 1, "name": "User Count", "database_id": 1, "display": "scalar", "query_type": "native", "archived": false,
+			})
+		case "/api/card/1/query":
+			if r.Method != "POST" {
+				t.Errorf("expected POST, got %s", r.Method)
+			}
+			json.NewEncoder(w).Encode(map[string]any{
+				"data": map[string]any{
+					"cols": []map[string]any{
+						{"name": "count", "display_name": "Count", "base_type": "type/Integer"},
+					},
+					"rows": [][]any{
+						{42},
+					},
 				},
-			},
-		})
+			})
+		default:
+			t.Errorf("unexpected path %s", r.URL.Path)
+		}
 	})
 	defer server.Close()