Skip to content

fix: skip unknown transform ops instead of silently running base64_decode#35

Open
rdwr-taly wants to merge 1 commit into
devfrom
fix/transform-op-graceful-degradation
Open

fix: skip unknown transform ops instead of silently running base64_decode#35
rdwr-taly wants to merge 1 commit into
devfrom
fix/transform-op-graceful-degradation

Conversation

@rdwr-taly

Copy link
Copy Markdown
Owner

Summary

Fixes a HIGH-severity behavior bug. _normalize_transform_op silently rewrote an unknown/unsupported transform op to base64_decode and executed it, so a flow authored with a newer/unknown op ran the wrong operation against live 24/7 traffic and returned plausible-but-wrong output. For a demo / traffic-simulation tool whose credibility rests on "what ran is what you configured," silent mis-execution is the worst failure mode.

Fix

  • execute_transform_ops now skips an unknown op with a structured, machine-readable warning — a warnings[] entry {"type":"unsupported_transform_op","op","set","index","status":"skipped"} plus a TRANSFORM_OP_UNSUPPORTED op=... set=... index=... marker logged at ERROR (surfaced to the run log by the step handler) — and continues. The op's output variable is left unset.
  • _normalize_transform_op now raises on an unknown op (defense-in-depth) instead of downgrading to base64_decode.
  • Behavior: the skip itself does not halt the flow; a later hard reference to the unset variable still resolves as an undefined reference and halts as before (the skip neither invents new failures nor papers over missing data).
  • Aligns with the cross-app FlowMap graceful-degradation strategy: unknown step type / transform op / operator ⇒ skip-with-warning, never crash, never silently mis-execute.

Tests

6 new unit tests: op-level skip / no-downgrade, _normalize_transform_op raises, TransformOp-instance path, missing op/set, step-level no-halt, all-ops-unknown. Full unit suite: 79 passed, no regressions.

Audit note

Run artifacts produced by earlier CLI versions may contain values from an unintended base64_decode of an unknown op's first argument. See RELEASE_NOTES.md for how to audit.

🤖 Generated with Claude Code

…code

_normalize_transform_op silently rewrote an unknown/unsupported transform
op to base64_decode, so a flow authored with a newer op ran the WRONG
operation against live 24/7 traffic and returned plausible-but-wrong output.

execute_transform_ops now SKIPS an unknown op with a structured, machine-
readable warning (logged as TRANSFORM_OP_UNSUPPORTED and surfaced by the
step handler) and continues; the op's output variable is left unset.
_normalize_transform_op now raises on an unknown op as defense-in-depth.

Adds 6 unit tests (op-level skip/no-downgrade, normalize-raises, TransformOp
instance path, missing op/set, step-level no-halt, all-ops-unknown) and a
severity-flagged RELEASE_NOTES entry with an audit note.

Part of the cross-app FlowMap graceful-degradation strategy.

Co-Authored-By: Claude Opus 4.8 <[email protected]>

@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: 3f3feadb48

ℹ️ 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 flow_runner.py
"(refusing to silently substitute base64_decode).",
op_name, set_hint, index,
)
continue

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 Clear stale transform outputs when skipping unknown ops

When an unsupported op targets a variable that already exists in context (for example from staticVars or an earlier extraction), this skip leaves the old value intact. Downstream transform refs or request substitutions can then read that stale value instead of treating the skipped op's output as unset, so a flow authored with a newer transform can still send plausible-but-wrong traffic despite the warning. Clear or otherwise invalidate set_hint before continuing so skipped outputs cannot reuse previous data.

Useful? React with 👍 / 👎.

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