Skip to content
This repository was archived by the owner on Jul 16, 2026. It is now read-only.

fix(shadow_mode): preserve original_action for throttle decisions#145

Merged
levleontiev merged 4 commits into
mainfrom
fix/shadow-throttle-original-action
Apr 11, 2026
Merged

fix(shadow_mode): preserve original_action for throttle decisions#145
levleontiev merged 4 commits into
mainfrom
fix/shadow-throttle-original-action

Conversation

@levleontiev

Copy link
Copy Markdown
Contributor

Summary

  • Bug: shadow_mode.wrap() set original_action to "allow" for throttle decisions, losing the "throttle" action value
  • Fix: Replace the binary was_allowed and "allow" or "reject" expression with direct preservation of decision.action before it is overwritten
  • Test: Added Scenario AC-13 and the corresponding step definition to cover the throttle-in-shadow-mode path

Details

The old code in src/fairvisor/shadow_mode.lua:

decision.original_action = was_allowed and "allow" or "reject"

This expression is binary: it can only produce "allow" or "reject". When a cost_based rule with a throttle staged action is evaluated under a shadow-mode policy, _build_decision produces a decision with action = "throttle" and no .allowed field. In wrap(), was_allowed = ("throttle" ~= "reject") = true, so original_action becomes "allow" — not "throttle".

The original_action field is read by _finalize_decision in rule_engine.lua as effective_action, which determines the SaaS audit event_type. With the bug, a shadow throttle was logged as event_type = "request_rejected" with action "allow", rather than event_type = "request_throttled".

The fix simply captures decision.action (the actual value) before overwriting it:

decision.original_action = decision.action

This is safe for all existing cases:

  • action = "reject"original_action = "reject"
  • action = "allow"original_action = "allow"
  • action = "throttle"original_action = "throttle" ✓ (was broken)

Fixes #144

Test plan

  • All existing shadow_mode spec scenarios pass unchanged
  • New AC-13 scenario passes: throttle decision wrapped in shadow mode has original_action = "throttle"
  • lua-quality CI check passes
  • validate-config-and-policies CI check passes
  • e2e-smoke CI check passes

The wrap() function was using a binary expression
  decision.original_action = was_allowed and "allow" or "reject"
which always sets original_action to either "allow" or "reject",
lost the "throttle" action when wrapping a throttle decision in shadow mode.

Fix by assigning decision.action directly before overwriting it.

Fixes #144
…ion fallback

The first attempt used decision.action directly, which broke existing
tests where the decision table has no .action field (only .allowed).

The correct fix: use decision.action when it is non-nil (preserving
'throttle' correctly), and fall back to the was_allowed-derived
value only when decision.action is nil.

Fixes #144
@levleontiev
levleontiev merged commit 048b790 into main Apr 11, 2026
11 checks passed
@levleontiev
levleontiev deleted the fix/shadow-throttle-original-action branch April 11, 2026 04:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

shadow_mode.wrap: original_action is wrong for throttle decisions

1 participant