Skip to content

Harden the Datamailer send boundary so misconfiguration fails closed #305

Description

@alexeygrigorev

Harden the Datamailer send boundary so misconfiguration fails closed

PM-groomed specification

Reviewed 2026-09-02 against origin/main at 2d3c28d1b8d1c2f3844f47f664945b42849d5eb9.
This issue is a P0 security/operations hardening slice for the existing course-platform
Datamailer client. It does not authorize a provider, a send, a migration, or production
configuration change.

The issue is groomed for two independently testable slices:

  • H1 is a test-only deployment-contract correction and can be implemented independently.
  • H2 is a client-boundary correction and can be implemented independently once the owner
    decisions below are recorded.
  • H3 is narrowed/deferred. The raw four-directory rule cannot be accepted on current main:
    accounts/views/account_settings.py and accounts/views/email_preferences.py still import the
    legacy Datamailer preference adapter. The website-native preference migration belongs to
    #227, and the complete source-surface
    inventory belongs to #290. This issue must
    not claim either migration.

Normative authority and observed gap

  • _docs/PROCESS.md defines the groom → engineer → independent tester →
    PM acceptance → focused commit → local merge/push → on-call lifecycle.
  • Spec 01 requires network side effects after commit
    through durable jobs, makes email_app the logical delivery boundary, and prohibits direct
    Datamailer/SES calls from website request/service/job code.
  • Spec 06 makes email management Relay-proxied and
    gives the website no provider adapter, renderer, provider store, or direct-send fallback.
  • Spec 07 requires provider credentials to stay
    outside the website, safe redacted failures, and rollback that never re-enables Datamailer.
  • Spec 09 requires every outbound path to be
    disabled during rehearsal/import and treats Datamailer as read-only migration/history input.

At this base, deploy/task_definitions.py pins five disabled safety values for web, worker, and
migration tasks, including empty DATAMAILER_URL and DATAMAILER_API_KEY. The release test's
metadata-normalization scenario asserts only DATAMAILER_TRANSACTIONAL_DRY_RUN == "1", so the
load-bearing URL/API-key pins could be removed without that scenario failing. Separately,
course_management/datamailer/client.py constructs a configured requests.Session request for
any non-empty URL/API key/client/audience; the transactional dry-run flag does not cover all
certificate, recipient-list, reminder, campaign, contact/list, and preference paths.

In scope

H1 — assert the complete deployed safety contract

Extend the deployment-release test so the web, worker, and migration task definitions each contain
the exact reviewed SAFETY_ENVIRONMENT mapping after release metadata normalization:

DATAMAILER_SYNC_ON_USER_CREATE=0
DATAMAILER_OUTBOX_DISPATCH_IMMEDIATELY=0
DATAMAILER_TRANSACTIONAL_DRY_RUN=1
DATAMAILER_URL=
DATAMAILER_API_KEY=

The test must reject omission or mutation of each safety key, specifically URL and API key. Keep
the existing builder validation unchanged unless a test-only coverage gap requires a narrowly
justified adjustment. H1 adds no secret, environment override, URL, or provider configuration.

H2 — fail closed before network construction

Add one code-level disabled boundary at DatamailerClient.request() (or a shared configuration
boundary that every request unconditionally reaches). Under the disabled deployed contract, the
boundary runs before URL/header construction, requests.Session construction, request creation,
DNS, or socket access. If the client currently constructs a default session in __init__, make
that construction lazy or otherwise prove that disabled construction has no transport side effect.

The resulting behavior must satisfy all of these conditions:

  1. Supplying a non-empty URL/API key/client/audience cannot enable a Datamailer request in a
    deployed web, worker, or migration process.
  2. DATAMAILER_TRANSACTIONAL_DRY_RUN=1 remains distinct from disablement and cannot authorize a
    non-transactional send endpoint.
  3. Disabled execution produces a deterministic typed outcome with a stable safe reason/category.
    The reason must not contain URL, API key/bearer value, recipient, payload, response body, or
    other secret/PII.
  4. Existing callers keep their established safe behavior or the explicitly chosen typed behavior:
    no public traceback/500, no acknowledged provider work, and no retry loop that treats a
    disabled call as a provider attempt. The outbox path must classify the typed disabled outcome
    as terminal/disabled (or leave it safely unclaimed according to the chosen contract), never as
    a successful acknowledgement or ordinary transient retry.
  5. There is one choke point. No second client/helper may bypass it.

The recommended method boundary is block every Datamailer HTTP method in deployed runtime
(GET, POST, PUT, and DELETE). Datamailer history/status reads remain migration/reconciliation
input and are not an operational website-provider exception. A GET-only exception is allowed only
if the owner explicitly approves an exact path allowlist with tests proving no mutation and no send
endpoint reachability. “Block write verbs” alone is not an accepted boundary because contact/list,
preference, campaign, and transactional routes must all be classified.

H2 does not remove or port any current send path. The Relay delivery lifecycle and Datamailer
freeze/drain/retirement remain owned by #49,
#50, and
#74.

H3 — narrow static boundary only; strict rule deferred

The strict raw rule over email_app/, events/, accounts/, and jobs/ is not an acceptance
criterion for this issue. The known accounts preference imports remain an explicit exception
owned by #227; the complete source inventory and adopted-path policy remain owned by #290.

The owner may choose one of these dispositions before engineering:

Neither disposition permits a blanket legacy exemption or a claim that accounts is Datamailer-free.

Explicit non-goals

Dependencies and ownership

Required before H1/H2 engineering dispatch

  • PM records the owner decisions below and freezes the base/tree and affected request categories.
  • All verification uses synthetic fakes and denied transport. No Relay, Datamailer, SES, Mailchimp,
    AWS, DNS, or production authority is available or used.
  • The known scheduled-regression baseline is recorded separately; Harden the Datamailer send boundary so misconfiguration fails closed #305 evidence may not relabel
    unrelated failures as this issue's result.

Coordination, not H1/H2 blockers

Owner decisions required before dispatch

These are open decisions, not implied adoption. Engineering must not infer a different behavior.

  1. HTTP method boundary: approve the recommended all-method deployed block, or record the exact
    GET-only history/status path allowlist and its non-mutation/send-exclusion tests.
  2. Disabled-call typing/handling: approve a stable typed disabled outcome. Recommended shape is
    a redacted DatamailerDisabledError (compatible with existing safe request-error handling) with
    a stable reason code; non-strict callers preserve their safe no-op/audit behavior, while strict
    worker/command paths fail safely without acknowledgement or blind retry. If a safe response
    status/result is preferred, record the mapping for public, job, and management-command callers.
  3. Fake-only test opt-in: approve endpoint-shape tests using an injected synthetic fake session or
    equivalent explicit test transport. It must never be read from deployment environment/configuration
    and must fail if it reaches a real network. No operational enable switch is requested.
  4. No-provider gates: approve zero-network evidence as mandatory: fake session request() is a
    failure sentinel, URL/header/session construction is checked to occur after the guard, and
    tests use only synthetic credentials/recipients/payloads. No external provider, DNS, socket,
    AWS, browser, or production data is permitted.
  5. H3 disposition: choose narrow email_app/events/jobs AST coverage or defer H3 to Pin the adopted Datamailer source surface in a static inventory #290;
    strict accounts coverage remains deferred to Extend account email preferences for event and newsletter delivery #227/Pin the adopted Datamailer source surface in a static inventory #290 either way.

Allowed files and tests

Preferred implementation is limited to:

  • core/tests/test_deployment_release.py for H1;
  • course_management/datamailer/client.py and, only if required for the typed outcome,
    course_management/datamailer/client_types.py or a single adjacent exception module for H2;
  • course_management/datamailer_outbox_dispatch.py and/or
    course_management/datamailer_outbox_retry.py only if required to keep a disabled outbox event
    from being acknowledged or retried; this is boundary handling, not a new delivery workflow;
  • course_management/settings.py and/or deploy/task_definitions.py only if an explicit disabled
    setting must be defined/pinned; no live values or secret references;
  • focused existing Datamailer tests under courses/tests/, data/tests/, and
    studio_courses/tests/, plus a narrowly scoped new test file if no existing home is suitable;
  • for narrow H3 only, a standard-library AST checker and its test under scripts//scripts/tests/.

Do not edit callers, templates, migrations, provider packages, Relay code, or unrelated deployment
workflows. Any need outside this list stops implementation and returns the issue for re-grooming.

Required evidence:

  • H1 per-workload exact safety-map assertions plus mutation/omission cases;
  • H2 fake-session/no-network tests covering transactional, recipient-list, certificate, deadline,
    campaign, contact/list, and preference request shapes; incomplete config; dry-run-only behavior;
    one representative public caller; and one outbox worker path;
  • typed reason redaction assertions and proof that disabled work is not acknowledged or blindly
    retried;
  • narrow H3 AST checks only if adopted;
  • applicable uv-backed Make migration, quality, type, CI, and container checks. No migration is
    expected; discovering a model change is out of scope and stops the issue.

H2 has no intended rendered-template change. The independent tester recomputes the change-selective
plan, runs the graph-selected backend Playwright tier, and marks screenshots not_applicable only
when the graph proves no changed page/error surface requires a screenshot. If an error/page surface
changes, desktop and mobile screenshots are required.

Acceptance and lifecycle gates

The implementation starts only after the owner decisions above are recorded. The engineer works
uncommitted in an isolated worktree and posts the frozen base/head, plan digest, exact file list,
tests, and no-provider evidence. A separate tester independently recomputes the plan, verifies every
H1/H2 criterion and security boundary, runs the focused Django and graph-selected Playwright checks,
and posts a terminal tester-final PASS with required screenshots or an evidenced not_applicable.

The PM then evaluates the tester evidence and scope, and posts ## Product Acceptance with ACCEPT
or REJECT. Only after tester PASS and PM ACCEPT may the engineer create a focused commit whose body
contains Closes #305; the orchestrator performs the local --no-ff merge/push, and on-call alone
observes the resulting CI. No pull request, deployment, provider enablement, or live email is part
of #305.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P0Must-have or release-blockingbugSomething isn't workingdecisionOwner decision requiredemailArea: emailoperationsArea: operationssecurityArea: securitytestingArea: testing

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions