Skip to content

Fix action result key collisions and secret leak in substitute mode#61

Merged
JE-Chen merged 1 commit intodevfrom
fix/action-executor-keys
Apr 25, 2026
Merged

Fix action result key collisions and secret leak in substitute mode#61
JE-Chen merged 1 commit intodevfrom
fix/action-executor-keys

Conversation

@JE-Chen
Copy link
Copy Markdown
Member

@JE-Chen JE-Chen commented Apr 25, 2026

Summary

  • ActionExecutor.execute_action keyed results as f"execute: {action}", so two identical actions in one batch collided and the first result was silently overwritten. Now keyed as f"execute[{index}]: {display}", matching execute_action_parallel.
  • With substitute=True, the substituted action (with ${env:SECRET} already expanded) was being written into both the log line and the result-dict key, leaking secrets. Now the original un-substituted action is used for the key, the success log, and the dry-run payload log; only the callable itself receives the substituted payload.
  • Docs (en / zh-TW / zh-CN) and the HTTP server test updated for the new key format.

Test plan

  • pytest tests/test_action_executor.py tests/test_substitution.py tests/test_http_server.py tests/test_action_registry.py — 33 passed
  • Full suite: 741 passed / 8 skipped (pre-existing skips)
  • mypy automation_file/core/action_executor.py — clean
  • New regression tests:
    • test_duplicate_actions_do_not_collide — two identical echoes both surface in the result dict
    • test_substitute_does_not_leak_into_result_key${env:FA_EXEC_SECRET} literal stays in the key, expanded value only reaches the callable

execute_action's result dict keyed entries on f"execute: {action}", so two
identical actions in one batch would collide and the first result would be
overwritten. With substitute=True, the substituted action (with ${env:...}
already expanded) was also written into the log line and the result key,
exposing secrets pulled in via ${env:...}.

- Key results as f"execute[{index}]: {display}" so duplicate actions keep
  both results, matching execute_action_parallel's existing format.
- Track the original (un-substituted) action separately and use it for the
  result key, the success log line, and the dry_run payload log; only the
  executed callable receives the substituted payload.
- Update docs (en / zh-TW / zh-CN) and the http server test for the new
  key format.
- Add regression tests for both fixes in test_action_executor.
@JE-Chen JE-Chen merged commit 7a0072e into dev Apr 25, 2026
4 checks passed
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