Skip to content

Email templates: refuse edits to code-managed templates on the server #491

Closed

Description

@Silthus

Map: Map: send email from code with one PostHog call
Spec: Spec: code-managed email templates, section "Lock (later slices)", server part

Cast: Opus 5.5, high
Repository: PostHog/posthog

Question

Does PostHog refuse every edit to a code-managed template that is not an upload, and tell the caller why and how to change it in code instead?

Delivers end to end. One ownership policy in the messaging product, checked on the row locked by the save path, keyed on action and whole payload. On managed_by = code: update, partial update (including draft-only writes when PostHog#118664 is in), design and soft delete answer 403 immutable with detail, why, fix (naming the file when the source is known) and docs_url. Allowed: upload, send, make transactional, and a release PATCH whose only field is managed_by: "gui" from session, personal API key or OAuth callers. A release keeps the key; the next upload claims the template again. The MCP update and design tool descriptions say a code-managed template changes through upload.

Port and adapt services/code_ownership.py from the fork's feat/workflows-code-managed-read-only (ebdfa6b6be, b84d8d7116, 5941fb9486), including the TOCTOU re-check on the locked row.

Test seams. The template API: one refusal per blocked action (update, partial update, design, delete, and draft-only when PostHog#118664 is in) and one pass per allowed action (upload, send, release, then a normal update after release). An MCP-style call (personal key with x-posthog-client: mcp) gets the same refusal. Prior art: test_hog_flow_code_managed.py on the fork.

Write scope. products/messaging/backend/services/ (ownership policy), products/messaging/backend/presentation/views/message_templates.py, products/messaging/backend/tests/api/, products/workflows/mcp/email_templates.yaml (descriptions), generated files.

Proof.

  • Red: an update of a code-managed template succeeds.
  • Green: every refusal and pass above.
  • Local: upload a template, try a PATCH with a personal key, show the verbatim 403; release it, PATCH again, it saves; upload again, it is managed by code again.

Acceptance criteria

How to build (map #461)

  • /tdd red-green-refactor, outside-in, at the seams named above. Real Postgres, Valkey and ClickHouse; mock only what the seam section allows.
  • Repo skills apply where the diff touches their area: /writing-tests, /improving-drf-endpoints, /django-migrations, /adding-api-scopes, /implementing-mcp-tools, /writing-ui-components, /writing-kea-logics, /writing-user-facing-copy, /writing-code-comments.
  • Read code from upstream/master (PostHog/posthog). The fork's master is stale.
  • Public repo: no usage numbers, customer names, internal metrics, Slack content or security reasoning in code, commits or the PR. Fixture addresses use example.com. The private note the map points at is background only; never quote it.
  • Shipped means a draft PR handed off for review (see the map's Notes for remotes and branches). Never mark ready, enqueue or merge.

Activity

  1. self-assigned this
    on Oct 16, 2026
  2. Silthus commented on Oct 16, 2026

    @Silthus
    OwnerAuthor

    Runtime: T3 child task
    Task: node:delegated-task:command%3Amcp%3A955e36b3-3986-469c-b816-09cf656780af%3Adelegate-task%3Awf-email-impl-491-1
    Cast: Opus 5.5, high. Cap 180 minutes. Lands on spec/code-managed-templates.

  3. Silthus commented on Oct 16, 2026

    @Silthus
    OwnerAuthor

    Resolution

    Landed: bfe29ccc10d on spec/code-managed-templates (fast-forward 65d441ac6e9..bfe29ccc10d, one commit).
    Branch: spec/code-managed-templates/491. No PR: the ship ticket #498 opens it.

    What landed

    • One ownership policy, products/messaging/backend/services/template_ownership.py. On managed_by = code it refuses every write except the actions it names: upload, send, make_transactional, and a release. It keys on the action and the whole payload, so a write action added later is refused until someone allows it.
    • Two checks, one rule:
      • The viewset's check_object_permissions runs the policy on every non-safe request that reads the template. So update, partial update, soft delete (PATCH {"deleted": true}) and design are refused before body validation.
      • save_template runs it again on the row it locks. An upload that claims the template between the first read and the lock still wins. The upload has its own path (save_locked_template), so it never meets the check.
    • Refusal: 403 with {type: "invalid_request", code: "immutable", detail, attr: null, why, fix, docs_url}. fix names the file as far as the template records it: `path` in `repository`, `path`, the template's file in `repository`, or "the template's file". The view renders it in one handle_exception branch.
    • Release: a PATCH whose only field is managed_by: "gui" sets managed_by = gui under the row lock, with update_fields. It writes no workflow version and does not change the content. It keeps the key, so the next upload claims the template again. It needs the same workflow editor access as an update. Project secret API keys still can't reach partial_update.
    • Facade: save_template_fields and save_template_content take an intent: TemplateWriteIntent (action and payload), so every save says what it is. Also new: check_template_write, is_release and release_template. TemplateWriteIntent and TemplateManagedByCode are in facade/contracts.py.
    • Schema: update, partial_update and design declare the 403 response. managed_by's help_text explains the lock and the release. TemplateUploadErrorSerializer is renamed TemplateErrorSerializer (and UploadRefused is renamed TemplateRefusal), because uploads and edits now share it. The generated TemplateUploadErrorApi becomes TemplateErrorApi, and nothing outside generated files used it.
    • MCP: the workflows-update-email-template and workflows-patch-email-template descriptions say a code-managed template answers 403 immutable and changes through workflows-upload-email-template. Regenerated with hogli build:openapi.

    Proof

    Red (the new test file run before the change): 11 failed, 1 passed. Every refusal case fails because the update succeeds, for example the MCP-style call:

    E       AssertionError: {'id': '01a146c5-…', 'name': 'Renamed in PostHog', …}
    E       assert 200 == 403
    

    The release test fails on managed_by: it stays 'code' because the field was read-only. The one test that passes is the allowed-actions guard (send and make transactional), which also passes after the change.

    Green: test_code_managed_template_lock.py has 12 passed. The whole products/messaging/backend suite has 343 passed, nothing skipped.

    Seam Case Result
    Template API, session PUT, PATCH name, PATCH deleted, PATCH managed_by + name, PATCH design/ 403 immutable; name, hash and workflow version unchanged
    Template API, personal key with x-posthog-client: mcp PATCH name 403 immutable
    Template API fix with full source, path only, no source names the file it knows
    Template API UUID send with a phs_ key, make_transactional 202, 200
    Template API release, edit, upload, edit 200 gui with key kept, 200, updated and code again, 403
    Template API upload claims the template mid-request 403, name unchanged

    Mutation: removing the locked re-check in save_template fails test_an_upload_that_claims_the_template_mid_request_still_refuses_the_edit (1 failed, 11 passed).

    Local gate on the landed tree:

    • hogli ci:preflight --strict --against origin/spec/code-managed-templates: 0 failures. The pre-push hook ran it again on the push: 0 failures.
    • uv run mypy --cache-fine-grained .: Success: no issues found in 24086 source files.
    • tsgo --noEmit --checkers 1 in frontend/: exit 0, no errors.
    • pnpm --filter=@posthog/mcp run typecheck: exit 0. lint-tool-names: pass. Tool schema snapshot test: 3 passed.
    • hogli build:openapi: exit 0, regenerated output committed.

    Local proof. Django ran from this worktree on a scratch database cloned from a migrated test database, with a personal API key (hog_flow:write) and a verified default sender. A wrapper forced only workflows-transactional-email-api on. Verbatim:

    curl transcript
    ### 1. upload a template by key
    $ curl -X PUT http://127.0.0.1:8491/api/projects/84/messaging_templates/by_key/welcome/ -H 'Authorization: Bearer phx_…' -d '{"name":"Welcome","subject":"Welcome, {{ variables.first_name }}","html":"<p>Hi {{ variables.first_name }}</p>","source":{"repository":"github.com/example/app","path":"emails/welcome.html","ref":"3f2c1ab"}}'
    {"id":"01a146cf-2573-0000-386b-24e10f9f7b30","key":"welcome","result":"created","content_hash":"v1:f74cd913f3aa2e5667a588f2865634e56693df77793ca594d18e9b5f89e3d2e6","managed_by":"code","source":{"repository":"github.com/example/app","path":"emails/welcome.html","ref":"3f2c1ab"},"transactional_hog_flow_id":"01a146cf-257c-0000-b78d-d638ff04d707"}
    HTTP 201
    
    ### 2. PATCH the code-managed template with a personal API key
    $ curl -X PATCH http://127.0.0.1:8491/api/projects/84/messaging_templates/01a146cf-2573-0000-386b-24e10f9f7b30/ -H 'Authorization: Bearer phx_…' -d '{"name":"Renamed in PostHog"}'
    {"type":"invalid_request","code":"immutable","detail":"This template is managed by code, so it can't be changed in PostHog.","attr":null,"why":"Its content comes from a file in a repository. The next upload of that file replaces changes made here.","fix":"Change `emails/welcome.html` in `github.com/example/app` and upload it again. To edit the template in PostHog instead, stop managing it in code first: send a PATCH whose only field is `managed_by` set to `gui`.","docs_url":"https://posthog.com/docs/workflows/transactional-email-api#immutable"}
    HTTP 403
    
    ### 3. release it
    $ curl -X PATCH http://127.0.0.1:8491/api/projects/84/messaging_templates/01a146cf-2573-0000-386b-24e10f9f7b30/ -d '{"managed_by":"gui"}'
    {"id":"01a146cf-2573-0000-386b-24e10f9f7b30","name":"Welcome","key":"welcome","managed_by":"gui","source_path":"emails/welcome.html","content_hash":"v1:f74cd913f3aa2e5667a588f2865634e56693df77793ca594d18e9b5f89e3d2e6"}
    
    ### 4. PATCH again
    $ curl -X PATCH http://127.0.0.1:8491/api/projects/84/messaging_templates/01a146cf-2573-0000-386b-24e10f9f7b30/ -d '{"name":"Renamed in PostHog"}'
    {"id":"01a146cf-2573-0000-386b-24e10f9f7b30","name":"Renamed in PostHog","key":"welcome","managed_by":"gui","source_path":"emails/welcome.html","content_hash":"v1:f74cd913f3aa2e5667a588f2865634e56693df77793ca594d18e9b5f89e3d2e6"}
    HTTP 200
    
    ### 5. upload again
    $ curl -X PUT http://127.0.0.1:8491/api/projects/84/messaging_templates/by_key/welcome/ -H 'Authorization: Bearer phx_…' -d '{"name":"Welcome","subject":"Welcome, {{ variables.first_name }}","html":"<p>Hi {{ variables.first_name }}</p>","source":{"repository":"github.com/example/app","path":"emails/welcome.html","ref":"9e8d7c6"}}'
    {"id":"01a146cf-2573-0000-386b-24e10f9f7b30","key":"welcome","result":"updated","content_hash":"v1:f74cd913f3aa2e5667a588f2865634e56693df77793ca594d18e9b5f89e3d2e6","managed_by":"code","source":{"repository":"github.com/example/app","path":"emails/welcome.html","ref":"9e8d7c6"},"transactional_hog_flow_id":"01a146cf-257c-0000-b78d-d638ff04d707"}
    HTTP 200
    
    ### 6. PATCH after the upload claimed it
    $ curl -X PATCH http://127.0.0.1:8491/api/projects/84/messaging_templates/01a146cf-2573-0000-386b-24e10f9f7b30/ -H 'Authorization: Bearer phx_…' -d '{"name":"Renamed again"}'
    {"type":"invalid_request","code":"immutable","detail":"This template is managed by code, so it can't be changed in PostHog.","attr":null,"why":"Its content comes from a file in a repository. The next upload of that file replaces changes made here.","fix":"Change `emails/welcome.html` in `github.com/example/app` and upload it again. To edit the template in PostHog instead, stop managing it in code first: send a PATCH whose only field is `managed_by` set to `gui`.","docs_url":"https://posthog.com/docs/workflows/transactional-email-api#immutable"}
    HTTP 403
    

    Step 3 printed only the selected fields. Step 4 is the same call shape and shows the 200. The upload in step 5 answers updated because the release left the name and then the edit changed it.

    Calls made

    • Test seams, as the ticket named them: the template API (APIBaseTest, CDP mocked at internal_requests.post) and one MCP-style personal key call. The upload pass is covered by the release flow (re-upload of a released template) and the existing upload tests, which upload into code-managed templates.
    • Race test: it flips managed_by to code right after dangerously_get_object returns, the moment between the permission check and the row lock, as the prior art's test_hog_flow_code_managed.py does. A real two-transaction race would need TransactionTestCase.
    • Refusal type: invalid_request, not DRF's authentication_error, because the refusal is about the row, not the caller. This follows the prior art.
    • Release matches the exact body {"managed_by": "gui"} on partial_update, so a form spread into a PATCH never releases by accident. On a template that is not managed by code it changes nothing and answers 200. Before, the same body ran a full save and wrote a workflow version. managed_by stays read-only in the serializer, so {"managed_by": "code"} is still ignored on a GUI template and refused on a code-managed one.
    • MCP: the update tool's input schema has no managed_by (the field is read-only), so its description does not tell agents to release. The UI release lands with Email templates: read-only view of code-managed templates #492.
    • design order: the route now reads the template before validating the operations, so a code-managed template answers 403 before any 400 about the ops.
    • Not covered by the lock: rewrite_template_content (the asset URL rewrite management command) is an operator tool, not an edit path, so it stays as it is. Workflow edits stay possible, as the map records.
    • Existing test changed: test_a_publish_moves_the_replaced_hash_to_the_superseded_list edited a code-managed template. It now releases the template first, then edits it.
    • No review ran in this ticket. The ship ticket Ship code-managed email templates as one reviewed draft PR #498 owns review.

    Gaps

    Follow-ups

    • Email templates: one send route that names the template in the body, with a content hash #488 adds the new send route. It must add its action to ALLOWED_ACTIONS in services/template_ownership.py only if it routes through check_object_permissions on this viewset. A separate MessagingSendViewSet does not meet the policy. Expect a merge in message_templates.py (the UploadRefused → TemplateRefusal and TemplateUploadErrorSerializer → TemplateErrorSerializer renames) and in the generated files. Regenerate the generated files instead of merging them by hand.
    • Email templates: adopt an existing template by giving it a key #490 (adoption by PATCH {"key": ...}) is an update, so it is refused on code-managed templates. That is consistent: a code-managed template already has a key.
    • desktop API client was not regenerated locally (generator not installed). Backend CI regenerates it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions