Skip to content

Allow body-driven Email Routing rule verification#18

Merged
rogu3bear merged 1 commit into
mainfrom
fix/email-routing-forward-rule-bodies
May 20, 2026
Merged

Allow body-driven Email Routing rule verification#18
rogu3bear merged 1 commit into
mainfrom
fix/email-routing-forward-rule-bodies

Conversation

@rogu3bear

Copy link
Copy Markdown
Owner

Summary

  • allow email.routing_rule mutations that provide a full body payload to verify by recipient/enabled state instead of always requiring a Worker action
  • keeps the existing Worker-specific verification path for the normal --service <worker> flow

Why

maildesk-cf needed to temporarily reconcile direct forward Email Routing rules for operator delivery addresses. The surface already accepts --body, but post-apply verification assumed every rule action was worker, so valid forward-action payloads failed verification.

Verification

  • bash -n scripts/cf_mutate_email_routing_rule.sh
  • ./scripts/verify_static_contract.sh

Notes

This does not add a full first-class destination-address model. That remains tracked in #17.


report_file="$(printf '%s\n' "${mutation_report}" | tail -n 1)"
if [[ "${APPLY}" == "1" && "${status}" -eq 0 && -f "${report_file}" ]]; then
if [[ -n "${BODY_JSON:-}" || -n "${BODY_FILE:-}" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: Incomplete verification logic for custom BODY_JSON/BODY_FILE path

When a custom payload is provided via BODY_JSON or BODY_FILE, the verification only checks that a matcher exists for the address and that the rule is enabled. It does NOT verify that the rule's actions route to the expected WORKER_NAME (unlike the standard path at lines 158-168). This creates a security/logic gap: a rule could match the address but route to a different destination, and verification would still pass.

report_file="$(printf '%s\n' "${mutation_report}" | tail -n 1)"
if [[ "${APPLY}" == "1" && "${status}" -eq 0 && -f "${report_file}" ]]; then
if [[ -n "${BODY_JSON:-}" || -n "${BODY_FILE:-}" ]]; then
if jq -e --arg address "${RULE_ADDRESS}" '

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: RULE_ADDRESS may be empty in custom payload path

The custom BODY_JSON/BODY_FILE branch allows RULE_ADDRESS to be unset (build_payload at line 46-49 doesn't require it). If RULE_ADDRESS is empty, the jq filter will silently match nothing and verification will fail with a generic error message, making debugging difficult.

@kilo-code-bot

kilo-code-bot Bot commented May 20, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
scripts/cf_mutate_email_routing_rule.sh 140 Incomplete verification logic for custom BODY_JSON/BODY_FILE path - does not check worker actions

SUGGESTION

File Line Issue
scripts/cf_mutate_email_routing_rule.sh 141 RULE_ADDRESS may be empty in custom payload path, leading to confusing failure
Other Observations (not in diff)

No issues found in unchanged code.

Files Reviewed (1 file)
  • scripts/cf_mutate_email_routing_rule.sh - 2 issues

Fix these issues in Kilo Cloud


Reviewed by grok-code-fast-1:optimized:free · 100,075 tokens

@rogu3bear rogu3bear merged commit 5d0a704 into main May 20, 2026
3 checks passed
@rogu3bear rogu3bear deleted the fix/email-routing-forward-rule-bodies branch May 20, 2026 01:24
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