Skip to content

Fix original activity identity from public Server claims - #221

Merged
rmcdaniel merged 1 commit into
mainfrom
fix/activity-owning-identity-250-20261027
Oct 27, 2026
Merged

rmcdaniel merged 1 commit into
mainfrom
fix/activity-owning-identity-250-20261027

Conversation

@rmcdaniel

Copy link
Copy Markdown
Member

When PHP SDK 3.6.0 receives a normal public activity claim, ActivityContext::workflowId and runId remain null because the constructor reads the Server's internal names. Real published-Server acceptance caught this when the guarded own-run signal example refused to send, even though the retry snapshot and original attempt numbers were correct.

Read the public workflow_id and run_id keys first, preserving explicit null and invalid/missing identity. Retain the internal-name aliases for existing constructor callers and local-parent metadata. No Server contract or retry scheduling changes. Prepare the 3.6.1 patch metadata.

Regression fixtures now use the public claim shape in ordinary and cooperatively supervised workers. Constructor controls cover public precedence, explicit null, legacy aliases, invalid identity and a partially missing identity. The new identity controls fail on the published 3.6.0 context. Full source qualification and real installed-package acceptance will be recorded before delivery is claimed.

Refs durable-workflow/.github#250. Follow-up to #220.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 27, 2026 02:51
@rmcdaniel

Copy link
Copy Markdown
Member Author

Qualified head a1b80ebe70c0f04780f1883e42b6128b8517bd2b for the 3.6.1 patch:

  • PHP 8.3.36 source suite: 2,859 tests, 14,896 assertions, 56 connected-environment skips, zero failures. PHPStan level 6, strict Composer validation, the production dependency boundary and release-registration identity passed.
  • The six new constructor identity controls fail in four cases against the original 3.6.0 ActivityContext source and pass in the candidate suite. Ordinary and supervised worker tests now use the public claim field names. Existing inline/prepared-local controls remain passing.
  • CI 40440603271 succeeded, including the regression corpus, PHP/framework matrix, docs and package checks. Sonar 40440603255 and CodeQL succeeded. Sonar App check 121980097378 reports zero new/accepted issues and zero hotspots; its reported new-code coverage is 0.0%.

Published-package/actual-Server acceptance remains pending until 3.6.1 is published and normally installed. This patch preserves the old constructor aliases and uses public keys whenever present, including explicit null. It does not guess a different workflow or run.

@rmcdaniel
rmcdaniel merged commit 2a44993 into main Oct 27, 2026
42 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head review of a1b80ebe70c0f04780f1883e42b6128b8517bd2b:

  • The correction reads the public activity-claim keys workflow_id and run_id first.
  • array_key_exists preserves explicit public null/invalid values as unknown instead of falling through to a conflicting legacy alias. This is the important no-retargeting boundary.
  • Existing strict nonempty-string validation remains; zero-like string IDs remain valid, while non-string and empty identities remain unknown.
  • Internal-name aliases apply only when the corresponding public key is absent, preserving local-parent and 3.6.0 constructor compatibility without changing the Server contract.
  • Focused controls cover public values, legacy aliases, conflicting values, explicit null, invalid values, partial identity, ordinary workers, and cooperatively supervised workers. Four new controls fail against published 3.6.0 as expected.
  • Scope is limited to ActivityContext, focused tests, and 3.6.1 package metadata. All current hosted checks, including the PHP/Laravel/Symfony matrices, regression corpus, package smoke, CodeQL, and Sonar, are green.

Accepted for the security boundary: the published regression failed closed, and this patch restores the original owning identity without creating a fallback that could target another workflow/run. No Security merge/release gate or Server/Rust change is required; normal owner merge, publication, and installed acceptance remain authoritative.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Post-publication identity check for PHP SDK 3.6.1:\n\n- The GitHub release targets verified merge commit 2a44993, whose second parent is the accepted PR head a1b80eb.\n- Annotated tag 3.6.1 dereferences to that same commit. The tag-triggered Public Boundary run 40441439780 passed.\n- Packagist now lists durable-workflow/sdk 3.6.1 with both source and dist references equal to 2a44993; released composer metadata reports product-train 3.6.1.\n\nThis closes the source-to-registry identity portion of the Security review. It does not substitute for the pending normal installed-package/actual-Server functional acceptance, which remains with its owner. No additional Security gate.

@rmcdaniel

Copy link
Copy Markdown
Member Author

PHP 3.6.1 published-package context and original-run acceptance completed 2026-10-27 02:57–03:11 UTC.

  • Release 3.6.1, annotated tag 894ced4ed88ba482fe6157e2300bd4b6b4ad766b, resolves to signed merge 2a44993761b1b900d3f631c0b13dccb8b45ddf5e. Its tree equals qualified PR head a1b80ebe. Independent main CI 40441007665 and Sonar 40441007549 succeeded before publication.
  • A fresh normal Composer registry ZIP installation reports 3.6.1 with matching source/dist references. Five critical installed files match the release bytes, and all three runtime classes resolve inside normal vendor. No path repository, class override, Core or Laravel dependency was used. Ten retry-contract cases, original public identity, immutability/copy and legacy-constructor controls pass.
  • Actual published Server 2.11.3, image index sha256:923a8924ecad13485562d7af30dc87d7fd6d9d7d4758ec8113bac0766fa8603e, bundled Core 2.14.2, SQLite and the normal database queue worker completed eight workflows / fourteen callbacks across ordinary, cooperatively supervised, worker-session, inline-local and advertised prepared-local paths.
  • All three remote bounded cases preserve attempts 1/2/3, maximum 3, final checks false/false/true, the full received snapshot and the original workflow/run IDs. Without retry options, ordinary/supervised claims omit the policy and stay unknown. With worker-session affinity, the Server supplies an explicit one-attempt snapshot; the SDK preserves that received maximum and final check. Both local paths retain parent IDs without inventing a remote snapshot.
  • Both exact installed guide helper functions compiled and executed. Each successful remote callback sent one signal to its original selected run. This proves routing, not duplicate receipt admission. The separate unchanged three-call control still records three signals for one stable key on this Server, as documented in shared216 and Core877. That accepted feature remains open.

The 3.6.1 public quickstart contract and generated API are live. PR222 corrects the new guide's receipt claim and minimum identity version. All eight main-guide translations are drafted; their publication remains next. Python remains outstanding, so shared250 stays open.

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