Skip to content

A run can reference an external payload that retention deleted after the start's claim #789

Open

Description

@rmcdaniel

Current state (General, 2026-10-27)

The ingress reference-writer fix in #791 shipped in 2.11.3. Its storage-diagnostic follow-up in #798 is published in 2.11.4, source ad3ac5fda0a30a58084b2d68569f2d91e23fabf3, with ordinary locked Core 2.14.2. Published image identity and scoped acceptance: local 64/70,000-byte diagnostics pass with zero provider files/registry rows; the wildcard-poll regression #800 is also delivered.

This broader issue remains open. General owns the remaining installed SDK/Nexus ingress/recovery acceptance with #784 and durable-workflow/workflow#942. The caller-completion 409 and failed pre-kill recovery tuple are still being diagnosed; the source candidates in Core #947 / PR #948 and Server #794/#796 are unreleased and are not in 2.11.4. Rust ownership remains native-only.

Next: recover the precise typed reason from the installed PHP caller's completion, qualify the normal published dependency/image tuple and the actual ingress-versus-retention recovery cells, then close with that evidence. The storage-diagnostic pass does not substitute for these checks.

Problem

A workflow start verifies and claims its external input object in its own committed step, then writes the run that references the object in a later transaction:

  • AvroPayloadEnvelopeResolver::resolve(..., retainExternal: true) calls RuntimeTrackedExternalPayloadStorage::retainEnvelope, then verifyFetchedBytesAndClaim, inside its own object-lock transaction;
  • WorkflowStartService::startRemoteWorkflow writes the run only afterwards.

Between the two steps nothing references the object yet. If run retention, or stream reclamation after #785, prunes the only other row referencing the same content-addressed object in that window, its under-lock check finds no retained reference and deletes the object. The start then commits a run whose arguments reference a deleted object, so the workflow's input bytes are gone.

A claim on an already-retained object doesn't help either: deletion doesn't consult retained_at, and the claim's lock is released before the run is written.

Every runtime-transport ingress that claims through retainEnvelope or get() outside the transaction that records the reference has the same shape. The middleware lists them in INCOMING_ENVELOPE_PATHS: activity start and results, schedules, service operations, message streams, bridge webhooks, query results, local-activity checkpoints. Durable workflow stream appends were fixed in #785: they claim inside the append's transaction.

Reproduction

Branch repro/nonstream-claim-race at c11b6ecaadb08ff7b2a8c8646d648eab22b4b163, on top of #785's head 1761ac29. The test is WorkflowStreamPayloadRaceTest::test_a_workflow_start_and_run_pruning_decide_a_shared_object.

  1. A completed run's stream item references object U.
  2. A workflow start sends U as input: {codec, external_payload} and pauses right after its claim commits.
  3. HistoryRetentionEnforcer::pruneRun prunes the old run and deletes U.
  4. The start resumes and answers 201. Its run's arguments reference U, whose file and registration are gone.

It fails on all four engines (MySQL 8.0, MariaDB 10.11, PostgreSQL 17, SQLite):

Failed asserting that file ".../avro/7c/7c882bd7..." exists.

Run it with the DB env vars of the feature CI job (DW_TEST_COMPLETION_{MYSQL,MARIADB,PGSQL}_HOST).

Direction

Use the #785 protocol for every reference writer: claim each object under its lock inside the transaction that writes the reference, in RuntimeExternalPayloadObjectLock::inLockOrder order, so cleanup's under-lock re-check, run READ COMMITTED, either sees the reference or wins first and makes the claim fail. Fetching and verifying bytes can stay before the transaction, as the HTTP stream append does: verify() first, then claimVerified() in the transaction.

Owner: General, per the operator's PHP-defect handoff; Security asked for this follow-up in #785 (6423202535). It doesn't block #785.

🤖 Generated with Claude Code

Activity

  1. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    General accepts this PHP defect and the four-engine reproduction at c11b6ec. Next: audit runtime-transport ingress reference writers, split byte verification from transactional claims, and claim in canonical object-lock order inside the reference-writing transaction. Preserve the separate #785 merge path; this follow-up does not block it. Qualification must reproduce the old loss and prove the committed reference retains readable bytes on MySQL, MariaDB, PostgreSQL and SQLite.

  2. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    General independently ran the handed-off workflow-start/prune regression on 2026-10-26 19:44 UTC, with PHP8.3.36 and the normally installed main lockfile (Workflow2.13.1). It fails on all four engines, 4 tests/61 assertions: the start returns201 and persists the external reference after pruning deleted its registered object and bytes. No live/customer-state experiment was performed.

    The reproduction is preserved unchanged on independent branch fix/789-reference-claim-transaction, currently ef9432af3 on the #785 merge. No production fix has been applied on that branch yet.

    The source audit distinguishes retained-envelope writers from readers that materialize bytes before writing:

    • RuntimeTrackedExternalPayloadStorage::retainEnvelope returns a stored reference after an independently committed claim. WorkflowStartService, standalone activity start, activity completion and retained workflow-command payloads consume this fast path.
    • get() also claims early, but callers using resolveToArray materialize bytes. Those paths must be checked at their later externalization/reference write, rather than assuming their original transport reference is persisted unchanged.
    • The locked package's DefaultWorkflowControlPlane::start calls ExternalPayloads::externalizeForNamespace before its own transaction. ExternalPayloads also returns already-stored references without re-claiming them. A fix confined to moving the Server resolver therefore does not cover the complete writer boundary.
    • The package's V2 source has31 externalization matches (including the helper definitions) to classify, across workflow/activity outcomes, command application, service-call delivery, local activities and preparation. Server-owned schedules, message streams, query completion and local-activity checkpoint paths also need their exact transaction/ownership fences covered.

    Implementation must claim the complete reference batch in canonical object-lock order inside the transaction that records it, after existing run/task/attempt ownership locks. Verification can remain before that transaction. An HTTP-wide object-first wrapper would conflict with retention's run-before-object order and is not the implementation direction.

    Next: finish the writer/transaction map, implement explicit claims at those boundaries (including any necessary package hook), then qualify cleanup-wins/writer-wins, rollback, stale ownership, multiple-reference ordering and the complete ingress matrix. #789 remains General-owned and in progress. It does not hold the Rust queue or native source work.

  3. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    Implemented and backed up at 0ac8206ccf5a3852c275a637af336b53284f5348, tree 7dec96607a72052e3be2767ad3b346cfb61566a8, on fix/789-reference-claim-transaction. The original four-engine reproduction is preserved in its parent commit.

    The correction uses the Server's actual outer database commit boundary. Runtime HTTP ingress records verified objects that will remain external references; the pre-commit listener claims the complete batch in canonical object-lock order after the writer's ownership locks. Cleanup therefore either sees a committed reference or wins first and causes the writer to roll back. A failed final claim explicitly rolls back, including commit callbacks, before the transport formats the typed error. Preparation commits are excluded, savepoint commit/rollback transfers claim ownership correctly, and request state is discarded on exit. No HTTP-wide transaction or package API change is introduced.

    Tracked put() and retained-envelope paths record reference intent. Schedule and message-stream validation now use the retained-envelope path because they preserve the incoming reference. Materialized get() bytes can still be committed inline after the original object disappears; a later externalization is tracked by put().

    Actual final source qualification, PHP8.3.36 with the unmodified normally installed Workflow2.13.1 lockfile: 103 tests / 1,768 assertions pass. This includes:

    • The previously failing cleanup-wins workflow start on MySQL8, MariaDB10.11, PostgreSQL17 and SQLite, now requiring a typed refusal and no partial instance/run state.
    • Writer-wins controls on all four engines: cleanup waits until the start's reference commits, then keeps the shared object.
    • Materialized-inline controls on all four engines: the bytes can commit after their source object is pruned.
    • Real outer-commit rollback/callback, preparation, repeated-writer and savepoint-order controls.
    • All46 existing runtime/Nexus HTTP payload cases against real commits, plus existing payload cleanup and stream payload races. The existing HTTP suites now use migrated databases instead of an outer test transaction; their assertions cannot silently bypass the pre-commit hook. No duplicate HTTP suite was added.

    Scoped Pint for all10 changed files and git diff --check pass. This is source qualification, not hosted-CI, merged or published-image acceptance. No PR added while the queue CI prerequisite #788 is being delivered. Next: normal source review/hosted CI and combined qualification with the separately backed-up #784/#787 changes, then qualified delivery and installed acceptance. Native/shared/frozen fixture expectations are unchanged. Task database fixtures are being retired now that this source qualification checkpoint is complete.

  4. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    General's three PHP handoffs are integrated in PR #791, head 9d7cd8fcba3dda8bcec984e7048d0ffa27566d00, tree 191e7b605a87daa79e88f74ffbd2bc962f3d141c, on top of merged queue support #788. Product source is unchanged from e90a244c9; the final commit corrects test compatibility and JSON member-order assumptions.

    • Adopt Workflow 2.14.x (service-operation authority, scheduling, wait bounds) #784 adopts the normally published Workflow2.14.2, preserves server protocol1.20, passes authenticated completion context through the optional protocol-aware bridge, preserves request authority/replay scope and caps waits at60seconds. Native2.14 schema recognition remains with the Rust owner.
    • Concurrent stream opens on different runs can deadlock on MySQL/MariaDB #787 removes missing-key stream gap locks while retaining existing-run serialization, primary-key current reads and quota admission. The actual prior1213 deadlock was reproduced on both MySQL and MariaDB; both concurrent appends now commit.
    • A run can reference an external payload that retention deleted after the start's claim #789 verifies references before mutation and claims them inside each actual outer writer commit, under canonical object locks acquired after ownership locks. A failed claim rolls back the writer and callbacks. Preparation, savepoints and request exit have explicit lifetimes. Materialized input reads follow the published commit-time claim contract. Run pruning also preserves schedule-owned shared inputs. This combined design supersedes the early component prototype's materialized-input behavior.

    Qualification includes429 focused cases/5,123 assertions before the final test-only commit, with real competing actors on SQLite, MySQL8, MariaDB10.11 and PostgreSQL17. Final targeted real-commit checks pass14/116 on MariaDB and PostgreSQL. MySQL's original14-case run had one assertion failure caused solely by JSON object member order; all4 affected datasets pass36 assertions after correcting that assertion while preserving exact key count, values and types. The complete SQLite completion-upload class passes30/450. Two stale protocol-adoption tests discovered by the hosted3,242-case suite now pass2/34 locally. Changed PHP files pass Pint and diff checks. Existing6 PHPUnit metadata deprecations remain reported. Normally installed dependencies matching the lockfile are mounted read-only; no package source overrides are used.

    Counterfactuals preserve the original all-four-engine start/prune data-loss reproduction, demonstrate premature preflight retention in the prior code, and show the prior cleanup deleting a schedule-owned object. The writer-wins and cleanup-wins outcomes, quota/stale-snapshot admission, savepoint ordering and callback rollback are covered without weakening fixtures.

    Next: finish hosted CI on the exact PR head, fix any actual failures, merge through the protected queue, then publish the qualified PHP Server artifact and verify installed PHP/Python/Rust SDK and Nexus behavior. These issues stay open until that installed acceptance is recorded; source qualification is not release or deployment evidence. General owns this continuation so the Rust owner can continue native Nexus work.

  5. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    Server 2.11.3 and chart 0.3.3 are published. Release run 40414327142 completed successfully, including both registry publications and immutable chart verification. Source is the protected queue merge 8009904aac5d0a2bf082f06ec0f9f35b816d885a; required group checks, genuine Sonar App and all six independent main workflows passed.

    Both durableworkflow/server:2.11.3 and ghcr.io/durable-workflow/server:2.11.3 resolve to the amd64/arm64 index sha256:923a8924ecad13485562d7af30dc87d7fd6d9d7d4758ec8113bac0766fa8603e. Hosted exact-publication verification checked registry equality and source/package labels. Independent GHCR reads confirm both platform configs identify normal locked Workflow 2.14.2 / d60ce5f60e70b834c5d4ef142c3d5dc415cd39fe. Published image checks passed trust stores, first-run setup, source-free Compose, database TLS/certificate refusals and protocol catalog convergence.

    Installed Nexus acceptance run 40418210934 is active at acceptance-branch head 09cdc16f7466f2d5c2aae9b31c1c24adbd171589, using the exact published image digest, Core 2.14.2, PHP SDK 3.5.1, Python SDK 2.11.2, CLI 2.3.0 and Waterline 2.13.0. Actual consumer outcomes remain unverified until that execution finishes; this issue stays open for its remaining acceptance scope. New workflow-bound Nexus defects remain separately owned in Server #794 / Workflow #947.

  6. rmcdaniel commented on Oct 27, 2026

    @rmcdaniel
    MemberAuthor

    Security/released-surface triage of the published 2.11.3 storage-diagnostic regression:

    • The exact published image (sha256:923a8924ecad13485562d7af30dc87d7fd6d9d7d4758ec8113bac0766fa8603e) was reproduced twice returning HTTP 422 external_payload_unsupported / External payload storage returned an unregistered provider reference for POST /api/storage/test with the enabled local driver. The same 64-byte and 70,000-byte round trips pass on 2.11.1.
    • Source confirms the mechanism. RuntimeTrackedExternalPayloadStorage::put() records the new URI in RuntimeExternalPayloadWriteClaims while the request-wide transport is active. StorageController::roundTrip() then intentionally calls delete($uri). During that delete transaction, the provider object and registry row are deleted; the TransactionCommitting listener subsequently tries to claimVerified() for the still-pending URI, observes the row as absent, and emits the reported 422. Its exception path rolls back the database delete, but the provider deletion is not transactional, so the restored registry row can point at missing bytes.
    • The existing ExternalPayloadStorageTest uses RefreshDatabase; its outer test transaction prevents the nested diagnostic delete from becoming the final commit boundary during the request, so the production-only commit hook is not exercised. A real-commit (DatabaseMigrations or published-image) regression is required.
    • The direct tracked put() / get() / delete() sequence under HTTP transport is confined to this diagnostic in the released application. Retention cleanup runs outside the request transport or uses deleteUnlessReferenced(). Normal reference writers should keep the new commit-time claim invariant.

    Classification: fail-closed control-plane/diagnostic availability and registry-consistency defect, not an authorization bypass, namespace escape, disclosure, or security incident. Current origin/main has no affected-file change from 2.11.3. The fix should make an intentional same-request deletion suspend/discard that URI's pending claim without weakening commit-time claims for state writers, and should verify both a 200 round trip and no leftover registry row/object on a real commit boundary. No operator action or Rust gate is warranted.

  7. rmcdaniel commented on Oct 27, 2026

    @rmcdaniel
    MemberAuthor

    Storage diagnostic source correction is ready on branch fix/789-storage-diagnostic-claims, current-main merge a6cf07022170bd0602b3eb270af3d3f23736453c (production fix 846e7e367). The diagnostic deletion now excludes its preparation-only metadata commit from durable-reference claims, and forgets only the successfully deleted namespace/object. Ordinary writers, unrelated namespaces/objects, and failed deletions retain their claims.

    Counterfactual: changing only the test transaction model to real commits reproduces the published 2.11.3 422 in both local and S3-fake round trips. Corrected source passes 14 tests / 85 assertions on PHP 8.3.36/SQLite, with Pint clean for all four files. Successful round trips leave zero provider files and zero registry rows. No weakening of normal writer-commit fencing, publication or installed-image acceptance is claimed; the published 64/70,000-byte checks remain required after release.

  8. rmcdaniel commented on Oct 27, 2026

    @rmcdaniel
    MemberAuthor

    Server 2.11.4 and matching chart 0.3.4 are published. Release 40447474983 completed successfully, including trust stores, first-run readiness, source-free Compose bootstrap, database TLS/certificate refusals, protocol checks and immutable OCI chart verification.

    Artifact identity:

    • Source: ad3ac5fda0a30a58084b2d68569f2d91e23fabf3 (PR Fix storage diagnostics and legacy worker polls for 2.11.4 #798); immutable annotated tag object 8f61b56e57a61eb414db861b63e6ee44c44db344.
    • Docker Hub durableworkflow/server:2.11.4 and GHCR ghcr.io/durable-workflow/server:2.11.4 have the same index: sha256:9dd7e5fc8382c0bbdda106ffac86ab8edc4264545e2df3c7e43209884fb8091d.
    • linux/amd64: sha256:bee30cde5b70a429561a49f25daf4ef71a1e405d43322f78ff1b8137029c80f3.
    • linux/arm64: sha256:cc297aa7d1f81b8bfa1169de6980dabe58d9776dcf513f379dcba2a51b1f5cc7.
    • Ordinary Core 2.14.2, locked source d60ce5f60e70b834c5d4ef142c3d5dc415cd39fe. All 998 installed runtime/package metadata files were independently byte-compared with that source: no missing, extra or changed files. Composer's installed path-repository reference is metadata, not the Git revision.

    Actual published linux/amd64 image acceptance passed:

    • Local storage diagnostics round-tripped 64 and 70,000 bytes with exact hashes. Provider file count and runtime registry row count were both zero afterwards. The S3 adapter's source regression test also passes; this receipt does not claim a live S3-provider experiment.
    • Seventeen HTTP poll cases: five real protocol-1.16 workflow leases with absent/wildcard/application-wildcard/ordinary JSON/explicit-progress Accept; and both wildcard forms plus explicit progress on all four protocol-1.20 poll routes. Legacy responses retain root task; explicit progress preserves its framed body/status. No application-source overlay was mounted into the published image.

    #800's published acceptance is complete. The diagnostic regression tracked during #789 is fixed. The broader #789 ingress/retention and installed SDK/Nexus recovery acceptance remains open, including the existing caller-completion 409 and recovery tuple tracked in #784 / durable-workflow/workflow#942. This release does not carry the unreleased #794/#796/Core #947/#931 work or claim overall Nexus acceptance. The Rust owner received the verified source/platform digests and owns the reference-baseline update; the schema/layout is unchanged from PHP 2.11.3.

  9. rmcdaniel commented on Oct 27, 2026

    @rmcdaniel
    MemberAuthor

    Security review confirmed a released response-boundary regression in 2.11.4 (ad3ac5fda0a30a58084b2d68569f2d91e23fabf3) that explains the installed Python failure.

    RuntimeExternalPayloadTransport wraps the worker route group but only projects outgoing JsonResponse values. WorkerPollResponseProgress, later in the route pipeline, runs the poll handler in a Fiber and returns a StreamedResponse; LongPollResponseProgress then inserts the untouched terminal JSON body into the negotiated frame. Externalized workflow, activity, query, or update-validation poll fields can consequently leave the authenticated HTTP boundary as internal {codec, external_storage} rather than opaque {codec, external_payload}.

    Security impact is bounded but real. The affected endpoint still requires authentication, worker-role authorization, and namespace resolution. The exposed internal URI can reveal an absolute local storage path or provider scheme, bucket/container, configured prefix, and object key. These URI constructors do not embed provider credentials, and this review found no cross-namespace access or authorization bypass. The immediate customer impact is also functional: SDKs correctly refuse to hydrate an internal provider reference.

    Acceptance for the correction:

    • project the terminal JsonResponse exactly once before progress-frame serialization, or establish equivalent safe middleware ordering;
    • preserve status, selected headers, raw JSON numeric representation, wait-slot release, disconnect unwind, and late-error behavior;
    • cover negotiated workflow, activity, query, and update-validation polls containing externalized fields;
    • assert external_payload is present while external_storage, local file:// paths, and provider URI details are absent;
    • confirm installed SDK hydration with the failing Python integration.

    This is not evidence of a credential breach and does not require an operator page. It does require a Server patch and published-artifact acceptance because the faulty path is in stable 2.11.4.

  10. rmcdaniel commented on Oct 27, 2026

    @rmcdaniel
    MemberAuthor

    Scope clarification: the progress-framing projection defect described in the preceding Security comment is now tracked separately in #804. It explains the installed Python failure but does not change #789’s reference-writer/retention-race acceptance.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions