Skip to content

fix(chat): expression resolution hardening - #3149

Open
jordan-umusu wants to merge 5 commits into
mainfrom
fix/chat-expression-resolution
Open

fix(chat): expression resolution hardening#3149
jordan-umusu wants to merge 5 commits into
mainfrom
fix/chat-expression-resolution

Conversation

@jordan-umusu

@jordan-umusu jordan-umusu commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Scope of guarantee

This PR enforces per-parameter expression policies at the argument-resolution boundary: direct SECRETS expressions and template-input-propagated secret dependencies (including compound expressions and nested templates) are masked at protected durable sinks (case content/fields/comments, table rows, preset instructions), and workflow-authoring source (patch_ops, definition_yaml) is preserved unevaluated.

It does not track transitive dataflow: a secret that passes through an intermediate step result (steps.*), ACTIONS, or VARS reaches sinks unmasked. This boundary is deliberate — whole-step-result taint was tried and reverted because it masked entire enrichment responses (any step using a secret credential tainted its full output) — and is pinned by test_template_step_result_is_not_tainted_by_its_arguments. Policies apply only where authored source crosses into a sink parameter; materialized runtime values carry no policy metadata.

Full decision ledger: amendment comment on ENG-1572.


Summary by cubic

Hardened expression evaluation for chat, template actions, and durable metadata to prevent secret exfiltration. Masks secrets at sinks, preserves authored workflow source, and keeps step results inert across template boundaries.

  • Bug Fixes
    • Switched to sink-level masking with input provenance instead of tainting template state; secret-dependent inputs.* are tree-masked and compound templates are redacted before evaluation.
    • Redaction for mappings now pairs by key (order-insensitive) and rejects secret-dependent keys with clear errors; prevents key collisions during source substitution and redaction.
    • prepare_resolved_context partitions args by policy before expression collection and uses prepare_action_args so preserved subtrees are excluded and durable fields are redacted ahead of any secret fetch.
    • Propagated source_provenance through nested templates and unified step arg handling via TemplateExecutionState; UDF steps with RESOLVE skip source substitution, and step results remain inert runtime data.
    • Redacts secrets in preset and table metadata at root: ai.agent.* (instructions, name, description, new_slug) and core.table.* (columns/default, column, update).

Written for commit a210b9d. Summary will update on new commits.

Review in cubic

@jordan-umusu jordan-umusu added fix Bug fix agents LLM agents labels Jul 28, 2026
@jordan-umusu
jordan-umusu marked this pull request as ready for review July 28, 2026 21:43
@zeropath-ai

zeropath-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to a210b9d.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► tests/unit/test_executor_expression_policy.py
    Add extensive unit tests covering expression policy behavior, secret redaction, provenance, and template argument handling
► tests/unit/test_executor_service.py
    Extend tests for executor service integration with expression policy, secret handling, and template execution
► tracecat/executor/expression_policy.py
    Add new module implementing expression policy and related helpers

@jordan-umusu jordan-umusu added the priority:high High priority ticket label Jul 28, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d08d48738b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/agent/mcp/executor.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7410497d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/agent/mcp/executor.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a0bd8ab9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/executor/service.py Outdated
@jordan-umusu
jordan-umusu force-pushed the fix/chat-expression-resolution branch from 5a0bd8a to 530632a Compare July 28, 2026 22:45
@jordan-umusu

Copy link
Copy Markdown
Collaborator Author

@codex re-review

@blacksmith-sh

This comment has been minimized.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 530632ad3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jordan-umusu
jordan-umusu force-pushed the fix/chat-expression-resolution branch 2 times, most recently from f741049 to fa8a797 Compare July 28, 2026 23:19
@jordan-umusu
jordan-umusu force-pushed the fix/chat-expression-resolution branch from fa8a797 to e93f222 Compare July 31, 2026 21:52

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e93f22285e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/executor/expression_policy.py
Comment thread tracecat/executor/service.py Outdated
Comment thread tracecat/executor/expression_policy.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3d4cf7d30

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/executor/expression_policy.py Outdated
@blacksmith-sh

blacksmith-sh Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Found 3 test failures on Blacksmith runners:

Failures

Test View Logs
TestCatalogUrlCredentialValidation/test_scheme_less_url_rejected_via_catalog_type View Logs
TestCatalogUrlCredentialValidation/test_token_value_not_validated_as_url View Logs
TestCatalogUrlCredentialValidation/test_valid_url_accepted_via_catalog_type View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27d0dc1508

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracecat/executor/expression_policy.py

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a210b9db65

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ActionParameter("core.table.insert_rows", "rows_data"): _REDACT_SECRETS,
ActionParameter("core.table.update_row", "row_data"): _REDACT_SECRETS,
ActionParameter("core.cases.insert_row", "row"): _REDACT_SECRETS,
ActionParameter("ai.agent.create_preset", "instructions"): _REDACT_SECRETS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Redact persisted preset base URLs

When a workspace-chat agent calls ai.agent.create_preset or ai.agent.update_preset with base_url: ${{ SECRETS.api.KEY }}, the unlisted parameter defaults to RESOLVE; AgentPresetBase only imposes a length limit, so the plaintext is persisted. A later default ai.agent.get_preset call returns the full configuration without loading that secret into its mask set, exposing it to the model. Add base_url to the redaction policy for both preset mutations.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents LLM agents fix Bug fix priority:high High priority ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant