Skip to content

test(export): pin the table and tool-param loss the sanitized export accepts - #6613

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/b2-export-table-subblock-redaction
Aug 12, 2026
Merged

test(export): pin the table and tool-param loss the sanitized export accepts#6613
waleedlatif1 merged 2 commits into
stagingfrom
fix/b2-export-table-subblock-redaction

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR intentionally changes no behavior. It pins an existing trade-off in place, documents it, and removes a footgun. No redaction logic is touched.

#6591 turned on redactOpaqueCredentialInputs for the workflow export path. That closed a real leak: a bearer token pasted into an API block's Headers cell, and secrets in custom tool params, were reaching the v1/v2 export APIs and the in-app Export as JSON in plaintext.

Its unpriced cost shipped with it. An export/import round trip now returns API blocks with empty Headers and Query Params, and drops {{ENV_VAR}} references inside table cells even though the same reference in a password field survives. Nothing in the test suite described either side of that trade, so it could be flipped in either direction without anyone noticing — and an earlier audit did recommend reverting the flag.

  • adds round-trip fixtures (an api block with two table sub-blocks, an agent block with a custom tool) plus assertions pinning the current loss
  • records the trade on redactOpaqueCredentialInputs — which surfaces it governs, what is lost, why withholding beat per-cell heuristics, and the shape a future relaxation must take
  • deletes a duplicate sanitizeForExport in credential-extractor.ts
  • corrects two route TSDocs that misdescribed what the code does, and moves the canonical sanitization doc onto the symbol those docs link to

The tests guard in both directions

Not just "redaction still works" — reverting the flag now fails loudly. Setting redactOpaqueCredentialInputs: false in json-sanitizer.ts produces exactly 4 failures:

× redacts nested tool credentials from the public export payload
× withholds table values, including whole {{ENV_VAR}} cells, on the way out
× withholds every parameter of a tool with no authoritative codec metadata
× re-imports the withheld sub-blocks as empty rather than dropping or corrupting them

The first one is the point: it is executable proof that reverting the flag re-opens a secret leak. The other three pin the accepted cost, so anyone who decides the loss is unacceptable has to edit those expectations deliberately rather than delete the flag and ship.

The deleted duplicate was a real footgun

credential-extractor.ts exported its own sanitizeForExport that called sanitizeWorkflowForSharing with preserveEnvVars: true and not redactOpaqueCredentialInputs. It had zero production importers — every real caller uses the same-named but different function in @/lib/workflows/sanitization/json-sanitizer, which does pass the flag and returns a full envelope.

Its only consumer was a test suite, which therefore described a configuration nothing runs. Two identically-named exports where one silently omits a redaction flag is exactly the shape of a future leak. The suite now passes the real option set explicitly, and import-export-roundtrip exercises the genuine sanitizeForExport, so the copy cannot drift unnoticed.

Open question for the owner

This PR does not resolve the underlying tension, because it is a product decision, not an engineering one:

Is workflow export a faithful backup format, or a share-safe projection?

Those two goals conflict here and the answer decides any future change. Today the export path tries to be both and lands as a share-safe projection that people reasonably expect to be a backup. If it is a backup, the fix is a per-sub-block opt-in that fails closed for tables added later — never a wider default. If it is a share-safe projection, the current loss is correct and the gap is discoverability: nothing in the product or the logs tells a user their tables came back empty; they find out by re-importing.

Worth noting the first relaxation candidates are already visible in the new fixture — api.params (query params) and response-shaping header tables are not credential sinks.

Recommended next action: discoverability, not relaxation

Independent of how that question is answered, the concrete follow-up I'd recommend is telling the user what was withheld — surface a warning on export (and on import) that table sub-blocks and unauthoritative tool params came back empty and must be re-entered.

That is the right next step because it is the only option with no security trade: it does not widen what is exported by a single byte, so it cannot re-open the leak #6591 closed, and it works whichever answer the product question gets. It converts a silent loss into an informed one. Today a user discovers the loss by re-importing a workflow and finding empty Headers — which is also why this reached a release unnoticed.

Relaxing the redaction (a per-sub-block opt-in that fails closed) is the larger, riskier change and should wait on the product decision. Discoverability should not.

Type of Change

  • Chore / test + docs (no behavior change)

Testing

35/35 passing across import-export-roundtrip, credential-extractor, and export-workflow. Bidirectional guard verified by flipping the flag and confirming 4 failures, then restoring. bun run type-check clean, biome clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…accepts

#6591 enabled `redactOpaqueCredentialInputs` on the workflow export path,
closing a real leak. It also made export lossy for tables and unauthoritative
tool params, and nothing pinned that trade in either direction.

Adds round-trip fixtures (an api block with two table sub-blocks, an agent
block with a custom tool) plus assertions for the current loss, records the
security/usability trade on the flag that governs it, and deletes a duplicate
`sanitizeForExport` in credential-extractor that omitted the redaction flag
and had zero production importers.

No behavior change.
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 12, 2026 8:42am

Request Review

@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test and documentation only; no production sanitization or export behavior is changed. Removing the unused duplicate reduces future footgun risk rather than introducing it.

Overview
Pins and documents the intentional lossy export trade-off from #6591 — no redaction behavior changes.

Adds round-trip fixtures (API tables + custom-tool params) that assert whole table values and unauthoritative tool params are withheld, including {{ENV_VAR}} cells. Reverting redactOpaqueCredentialInputs now fails those tests loudly in both directions (leak reopen vs. accepted cost).

Removes the unused duplicate sanitizeForExport in credential-extractor that omitted the redaction flag (a footgun the old suite was testing). Points that suite at the real export option set, and expands docs on redactOpaqueCredentialInputs plus the v1/admin export route TSDocs so zip/json and sanitized vs raw are no longer misdescribed.

Reviewed by Cursor Bugbot for commit b6a3b38. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR documents the intentionally lossy workflow-export sanitization behavior, adds round-trip coverage for withheld table and custom-tool values, and removes an unused duplicate sanitizer wrapper.

  • Pins table-cell and opaque tool-parameter redaction through export/import tests.
  • Corrects export-route and sanitizer documentation.
  • Consolidates export sanitization around the canonical implementation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts Adds round-trip fixtures and non-vacuous assertions for table and custom-tool redaction; both previously reported test issues are resolved.
apps/sim/lib/workflows/credentials/credential-extractor.ts Documents opaque credential redaction behavior and removes the unused duplicate export sanitizer.
apps/sim/lib/workflows/credentials/credential-extractor.test.ts Updates sanitizer tests to use the production export option set explicitly.
apps/sim/lib/workflows/operations/export-workflow.ts Relocates and expands the canonical export sanitization documentation without changing behavior.
apps/sim/app/api/v1/admin/folders/[id]/export/route.ts Clarifies the behavioral difference between raw JSON and sanitized ZIP exports.
apps/sim/app/api/v1/workflows/[id]/export/route.ts Clarifies that exported workflow envelopes are sanitized and may re-import with empty values.

Reviews (4): Last reviewed commit: "test(export): make the env-ref leak swee..." | Re-trigger Greptile

Comment thread apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts Outdated
Comment thread apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts
Comment thread apps/sim/lib/workflows/operations/import-export-roundtrip.test.ts Outdated
The sweep asserted against a token the fixture no longer contained, so it
passed vacuously. Both the fixture and the assertion now read one symbol,
which is the only form that cannot drift.

Also types the re-imported block lookup instead of casting through any.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b6a3b38. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b6a3b38. Configure here.

@waleedlatif1
waleedlatif1 merged commit 2054947 into staging Aug 12, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/b2-export-table-subblock-redaction branch August 12, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant