Skip to content

feat(email): allow server-side principal-less system sends on send-email (#3534) - #3535

Open
chubes4 wants to merge 1 commit into
mainfrom
fix/3534-system-send-direct
Open

chubes4 wants to merge 1 commit into
mainfrom
fix/3534-system-send-direct

Conversation

@chubes4

@chubes4 chubes4 commented Sep 21, 2026

Copy link
Copy Markdown
Member

Mechanism

Closes #3534. Direct datamachine/send-email calls with no acting principal currently fail (email_auth_ref_required, or email_mailbox_forbidden with an explicit default ref), which has been breaking registration/welcome emails and anonymous contact-form mail since the #3218 gate landed. The queued path already works for the same callers because it stamps issuer_type: system and EmailAuth::can_use_default() honors the principal_less_system + _trusted_execution context.

This PR exposes that existing path to direct sends — no new policy branch, no new mailbox model:

  • send-email input schema gains system (boolean, default false), documented as server-side only.
  • In execute(), when system: true and there is no acting user and no acting agent, the sender is resolved via EmailAuth::resolve_mailbox_for_principal( $account, 'send', array( 'principal_less_system' => true ) ) — the same can_use_default() branch the queued worker's system path reaches. The resolved mailbox identity becomes from_email / reply_to / from_name (instead of the admin_email fallback).
  • auth_ref still takes precedence when both are supplied (pre-existing branch untouched).

REST / MCP / agent guard

The flag is stripped from input before normalization (it never reaches the payload, logs, or grant material) unless every eligibility check in SendEmailAbility::canSendAsSystem() holds:

  1. PermissionHelper::acting_user_id() <= 0 — a logged-in non-manager passing system: true gets the flag stripped and is denied by the existing email_auth_ref_required gate; managers keep their pre-existing legacy-sender behavior (the flag never applies to them — their sender stays the admin_email fallback).
  2. null === PermissionHelper::get_acting_agent_id() — MCP / agent bearer-token callers are always stripped.
  3. REST-originated requests (REST_REQUEST) are stripped unless executing inside the trusted PermissionHelper::run_as_authenticated() seam.

Point 3 deserves explanation: the primary reported consumer (registration/welcome email in extrachill-users) runs ec_send_email() inside run_as_authenticated(0) during REST-handled registration POSTs. A blanket REST_REQUEST strip would keep the reported bug unfixed. External REST/MCP tool callers can never reach the honored branch anyway: checkPermission() denies any caller without a principal, and any caller with a principal is covered by checks 1–2. This mirrors how _mailbox_grant is protected — not by refusing REST requests, but by making the capability producible only from trusted server-side code paths.

System mailbox option

New datamachine_email_system_mailbox site option (string account name, default default), consulted by that path. Naming a dedicated transactional account later requires no code change. Note that resolution goes through the existing can_access() policy untouched: a named site-scope account is currently denied under a principal-less context (email_mailbox_forbidden) — exactly as the queued system path behaves today — so only default is usable as the system mailbox until a policy change deliberately extends principal-less access to site-scope named accounts (deliberately out of scope here, and away from #3510's active rebase). The option is forward-plumbed so that change lands without touching this path.

CLI: wp datamachine email mailboxes

Redacted operator listing (account, scope, owner id, IMAP host, IMAP user, plus a system marker column). Backed by a new EmailAuth::get_mailbox_index() — the storage enumeration lives in the provider; the CLI never parses datamachine_auth_data by hand. Passwords/tokens are never included (asserted by test).

Tests

New tests/send-email-system-flag-smoke.php (pure-PHP runtime smoke, real EmailAuth + real SendEmailAbility, 35 assertions), covering:

  • (a) system: true with no principal authorizes the default mailbox and dispatches with the mailbox identity.
  • (b) system: true from a logged-in non-manager (and from an acting agent) is denied with the existing email_auth_ref_required code; manager path unchanged.
  • (c) system is stripped for REST origin outside the trusted seam, and honored inside it.
  • The system-mailbox option is consulted (unconfigured name → auth_ref_unresolved; site-scoped named account → existing email_mailbox_forbidden policy).
  • get_mailbox_index() / CLI redaction contract (no credential fields, no secrets in output).

All pre-existing email-area smoke tests still pass (send-email-template-smoke 121 assertions, named-mailbox-delegation-smoke, named-mailbox-security-contract-smoke, abilities-send-email-load-order-smoke, cli-global-options-smoke, email-reply-sent-copy-smoke, legacy-email-upgrade-auth-smoke, auth-ref-handler-config-smoke). php -l clean on all changed files; homeboy review audit --changed-since origin/main --profile pr → pass.

Lint note: homeboy review lint --changed-only reports 6 findings — all on lines byte-identical to origin/main (verified by linting main's own file versions, which produce the same findings at main's line numbers). No new findings are introduced by this diff; fixing the pre-existing patterns is out of scope here to keep the diff tight for #3510's parallel rebase.

Scope vs #3510 / extrachill-network#235

  • Cross-ref extrachill-network#235 (consumer fix: ec_send_email() passes system => true when there is no acting user).
  • Cross-ref PR fix: gate email abilities on mailbox ownership, not just use_tools #3510 (ownership gating for per-user mailboxes): this PR does not alter fix: gate email abilities on mailbox ownership, not just use_tools #3510's ownership gating. No permission callback in any ownership-gating file was modified — the only EmailAuth change is an added read-only get_mailbox_index() method; can_use_default(), can_access(), and resolve_mailbox() are untouched.
  • Queued-path note: a system key forwarded through a queued payload is inert — non-manager issuers can't queue without an auth_ref (queue gate denies first), and with an auth_ref the pre-existing auth_ref branch takes precedence in execute().

Closes #3534

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

Labels

None yet

Projects

None yet

1 participant