feat(api): accept both keepiq and doriath on the machine API wire contract by rjzondervan · Pull Request #655 · ConductionNL/keepiq

rjzondervan · 2026-09-08T10:31:35Z

Closes the last three doriath identifiers, which are all published wire contract rather than internal storage: the JWT aud claim, the discovery path, and the envelope format name.

Each gets the treatment its direction allows, and all three are pinned to one deadline.

Audience — dual-accept (inbound)

aud is offered by the client and honoured by the server, so accepting a second value is invisible to everyone. Rejecting doriath outright is not available to us: every application registered before the rename signs it into an RS256 assertion with a private key this server does not hold and cannot re-sign, so a flip is a fleet-wide credential outage whose fix lives in each consumer's configuration.

Accepting the pair is additive, so it ships inside the current apiVersion without violating the "never an in-place mutation of an existing version" rule in secret-store-api.

This does not widen the confused-deputy guard RFC 7519 §4.1.3 exists to provide — both values name this app, and no other service in the fleet claims aud=doriath.

Every assertion accepted on the deprecated value is logged with its iss, so the set of consumers still to migrate is observable rather than guessed at.

Also fixes a latent interop bug: the old check was (string)$claims['aud'] !== EXPECTED_AUDIENCE, but RFC 7519 §4.1.3 permits aud to be an array — a conformant client sending aud: ["keepiq"] stringified to "Array" and was rejected. The claim is now normalised to a list.

Discovery path — dual-serve (inbound)

Served at both /api/v1/app/.well-known/keepiq (canonical) and /api/v1/app/.well-known/doriath, byte-for-byte identical. The document publishes discoveryPath and deprecatedDiscoveryPaths[].removedInAppVersion, so a consumer configured with the old URL can re-point itself. This is the one URL a consumer holds by hand — everything else it uses is derived from what this returns. Each hit on the old path is logged.

Envelope format — announce only (outbound)

This one cannot be dual-valued, and pretending otherwise would be the mistake. Exactly one string goes into format, and whichever it is, every consumer pinned to the other rejects the secret. So the server publishes the successor as upcomingEnvelopeFormats[] (value, replaces, emittedFromAppVersion) so consumers can be taught to accept both names ahead of time, after which the switch is a non-event. It is deliberately not listed in envelopeFormats, which declares only what is actually emitted.

Deadline

All three point at one shared constant, Application::PRE_STABLE_COMPAT_REMOVED_IN = '1.0.0', rather than a future apiVersion. Nothing stable has shipped, so there is no released contract a version bump would protect; apiVersion stays at 1 through their removal. The spec now states that its "breaking changes ship as a new apiVersion" rule binds from the first stable release onward.

⚠️ Two assumptions to confirm: that 1.0.0 is the production marker (nothing in the repo states one), and that scoping the spec rule to post-stable is acceptable. Both are one-line changes — that is why the deadline lives in a single constant.

Verification

Live, after occ upgrade onto the consolidated migration:

.well-known/keepiq  200 ┐ identical documents
.well-known/doriath 200 ┘
audience "keepiq", accepted [keepiq, doriath], removedInAppVersion 1.0.0
envelopeFormats [doriath-machine-secret-v1] + upcoming keepiq-machine-secret-v1

1181 tests pass, phpcs 0 errors, psalm clean. Adds three requirements to secret-store-api.

🤖 AI disclosure: prepared with Claude Code (Opus 5). Commits carry Assisted-by: trailers. Reviewed and submitted by @rjzondervan; please reword this description in your own voice if preferred.

Adds `keepiq` as the canonical `aud` value while continuing to accept the pre-rename `doriath`, and pins the removal of the old value to apiVersion 2. Rejecting `doriath` outright is not available to us: every application registered before the rename signs it into an RS256 assertion with a private key this server does not hold and cannot re-sign, so a flip is a fleet-wide credential outage whose fix lives in each consumer's configuration. Accepting the pair is ADDITIVE, so it ships inside the current apiVersion without violating the "never an in-place mutation of an existing version" rule in openspec/specs/secret-store-api/spec.md: every existing consumer is unaffected, and a self-configuring one reads the canonical value from discovery and converges on its own. Only the removal is breaking, which is what apiVersion 2 is for - the same coordinated bump that retires the `doriath-machine-secret-v1` envelope name and the `.well-known/doriath` path. This does not widen the confused-deputy guard RFC 7519 4.1.3 exists to provide. Both values name this app, and no other service in the fleet claims `aud=doriath` - the only other hits are the pre-rename copy of this same codebase in apps-extra/doriath. Every assertion accepted on the deprecated value is logged with its `iss` and the retiring apiVersion, so the set of consumers still to migrate is observable before the removal lands rather than guessed at. Fixes a latent interop bug in the same check. RFC 7519 4.1.3 makes `aud` either a string or an array of strings, but the old comparison was `(string)$claims['aud'] !== EXPECTED_AUDIENCE`, so a conformant client sending `aud: ["keepiq"]` stringified to "Array" and was rejected. The claim is now normalised to a list and accepted when any member names this instance. Discovery publishes `audience` (send this), `acceptedAudiences` (honoured) and `deprecatedAudiences[].removedInApiVersion`. Verified live: apiVersion 1, audience "keepiq", both values accepted. Assisted-by: ClaudeCode:claude-opus-5

…ssor Completes the pre-rename wire identifiers, and re-anchors all three deadlines away from "apiVersion 2". DISCOVERY PATH is inbound, so it gets the same treatment as the audience: the document is now served at both `/api/v1/app/.well-known/keepiq` (canonical) and `/api/v1/app/.well-known/doriath`, byte-for-byte identical, and publishes `discoveryPath` plus `deprecatedDiscoveryPaths[].removedInAppVersion` so a consumer configured with the old URL can re-point itself. This is the one URL a consumer holds by hand - everything else it uses is derived from what this returns - so serving both costs nothing and removes the need for a change window. Each hit on the old path is logged. ENVELOPE FORMAT cannot get the same treatment, and pretending otherwise would be the mistake here. Audience and path are inbound: the consumer offers a value and the server decides whether to honour it, so accepting a second one is invisible to everyone. The envelope format is outbound - exactly one string goes into `format`, and whichever it is, every consumer pinned to the other rejects the secret. No server-side change makes that flip safe. So the server does the only useful thing available to it: publishes the successor as `upcomingEnvelopeFormats[]` (value, replaces, emittedFromAppVersion) so consumers can be taught to accept BOTH names ahead of time, after which the switch is a non-event. The successor is deliberately NOT listed in `envelopeFormats`, which declares only what is actually emitted - advertising a format nothing writes is a claim a consumer could act on. RE-ANCHORED THE DEADLINES. All three previously pointed at "apiVersion 2", which frames a pre-production cleanup as a post-release migration. This app has never shipped a stable release, so there is no released contract a version bump would protect. The shims are now pinned to a single shared constant, Application::PRE_STABLE_COMPAT_REMOVED_IN = '1.0.0', and `apiVersion` stays at 1 through their removal. The spec's "breaking changes ship as a new apiVersion" rule is stated to bind from the first stable release onward, which is exactly when these stop existing. Verified live: both paths return 200 with identical documents; audience "keepiq" canonical with both accepted; successor announced, not emitted. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T10:41:13Z

Quality Report — ConductionNL/keepiq @ `f152b5a`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	❌
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

Quality workflow — 2026-09-08 10:41 UTC

Download the full PDF report from the workflow artifacts.

Answers phpmd's CouplingBetweenObjects on JwtAssertionVerifier by moving a concern out of it rather than raising the threshold. The verifier was at the limit before this branch touched it, so anything added tipped it over. Rather than shave a dependency to fit, the audience decision now lives in AudiencePolicy, which owns the constants, the accept/deprecate logic and the reporting. JwtAssertionVerifier returns to exactly its state on development, minus the one audience check it no longer performs. That split is worth making on its own terms. The verifier deserializes an assertion and checks its claims are well-formed and in-date; WHICH audience strings this deployment answers to, and for how much longer, is a published contract decision that also has to appear in the discovery document. Those are different jobs with different reasons to change, and keeping them together is what had the verifier reaching into another service's constants. JwtAuthService asserts the audience immediately after reading the claims, so the check still happens before a token can be issued. DiscoveryController now reads the constants from AudiencePolicy. phpmd, phpstan and phpcs clean; 1181 tests pass; both discovery paths verified live to serve identical documents with the audience set intact. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T12:22:45Z

Quality Report — ConductionNL/keepiq @ `7ee187b`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					❌

Quality workflow — 2026-09-08 12:22 UTC

Download the full PDF report from the workflow artifacts.

@e2e

Hydra gate-113 (exclusion-evidence) fails on every PR against development: openspec/specs/application-mgmt/spec.md:158 @e2e exclude -> SecretRequestServiceTest::testAdminRevokeDeletesTheUnfilled ApplicationPlaceholder not found Pre-existing and unrelated to this branch, but it blocks the gate, and the correction is unambiguous. All four cited methods - testAdminRevokeDeletesTheUnfilledApplicationPlaceholder, testAdminRevokeNeverDeletesAFilledApplicationSecret, testAdminRevokeWillNotDeleteAnotherApplicationsSecret and testRevokeForApplicationRefusesARequestOfAnotherActor - live in ApplicationRequestAdminServiceTest. SecretRequestServiceTest still exists, which is why the citation reads as plausible; it just holds none of them. The exclusion claimed verification it could not deliver. It can now. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T12:31:25Z

Quality Report — ConductionNL/keepiq @ `5cad9ba`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

Quality workflow — 2026-09-08 12:31 UTC

Download the full PDF report from the workflow artifacts.

Review point from #655: presentedValues() accepted any scalar and cast it to a string, so `aud: 123` and `aud: true` were normalised into "123" and "1" before matching, and a mixed array was filtered down to the members that happened to parse. RFC 7519 section 4.1.3 defines `aud` as a StringOrURI or an array of them. Anything else is a malformed assertion, and the claim is now read as such: non-strings present no audience at all, and an array containing any non-string or empty member is rejected WHOLE rather than filtered. The whole-array rule is the part that mattered. Measured against the previous implementation, of six malformed shapes now covered, three changed verdict: ['keepiq', 123] was accepted, now rejected ['keepiq', ['keepiq']] was accepted, now rejected ['keepiq', ''] was accepted, now rejected Filtering a mixed array authenticated the caller on its well-formed remainder, which is precisely what a malformed claim must not achieve. The bare scalars (123, true, 1.5) already failed, but only by accident: they coerced to values that matched nothing. That is a fact about the accepted set containing no numbers, not a property of the check, and an auth path should not lean on it. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T13:02:38Z

Quality Report — ConductionNL/keepiq @ `4daadb7`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

Quality workflow — 2026-09-08 13:02 UTC

Download the full PDF report from the workflow artifacts.

WilcoLouwerse

0 blockers, 2 concerns require attention before merge — the deprecation deadline (PRE_STABLE_COMPAT_REMOVED_IN) is documentation-only with nothing enforcing it, and the new discovery-document audience fields are unasserted by any test. The audience-membership logic itself, discovery byte-identity, deprecation logging, and envelope-format announce-only behavior all check out against the actual code.

…ence Wilco's review on #655: two concerns, three minors. CONCERN - THE DEADLINE WAS DOCUMENTATION ONLY. PRE_STABLE_COMPAT_REMOVED_IN was read in three places and every one of them only PUBLISHED it to consumers as removedInAppVersion / emittedFromAppVersion. Nothing compared it against the running version, so the three shims - including acceptance of a legacy audience on the token-exchange path - would have survived 1.0.0 in silence unless somebody remembered. A promise made to integrators that the codebase could not keep. PreStableCompatDeadlineTest fails the build once info.xml reaches the declared version while any shim is still present, naming each one. Below the deadline it asserts the OPPOSITE - that all three are still findable - so a rename cannot leave it silently guarding nothing, which is the failure mode the test exists to prevent. A second test pins the three published deadlines to the shared constant so they cannot drift and tell consumers different dates. Verified by temporarily setting <version> to 1.0.0: the build fails and lists all three shims. Not a runtime check, deliberately. The shims are correct while they are supposed to exist, and refusing to boot an instance over a version number is worse than refusing to ship one. CONCERN - THE PUBLISHED AUDIENCE FIELDS WERE UNASSERTED. Acceptance mechanics are covered thoroughly in JwtAuthServiceTest, but what a consumer is TOLD to send is a different surface: a swapped constant or a dropped field would break self-configuring clients with every acceptance test still green. Two tests now cover the three fields, one of them asserting the canonical value is not also listed as deprecated - a copy-paste that would read as plausible while telling consumers to migrate away from the value they should adopt. MINORS. The DiscoveryController class docblock still opened on the old canonical path and repeated the superseded "renaming would break every consumer" rationale; it now describes dual-serving and points at the deadline test. readAcceptableClaims() still listed aud="doriath" as a required claim after the check moved out; it now names presence only. The spec sentence I introduced ran on so that "the first stable release" appeared to declare the API version; split into three. 1191 tests pass; phpmd, phpstan and phpcs clean. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T13:58:23Z

Quality Report — ConductionNL/keepiq @ `21863f5`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

Quality workflow — 2026-09-08 13:58 UTC

Download the full PDF report from the workflow artifacts.

WilcoLouwerse

Reviewed commit 9f165dc. The earlier audience-type, discovery-contract coverage, and deprecation-deadline findings have been addressed.

Two issues remain: object-valued audience claims can pass the new array handling, and deprecated-audience acceptance is logged before issuer/signature/replay verification completes.

The latest reported workflows pass; these findings concern cases the existing tests do not cover.

…cation Wilco's two P2 findings on #655. Both are cases the tests did not cover, and both undo something this PR claimed to have established. OBJECT-VALUED AUDIENCES PASSED THE STRICT CHECK. readAssertionClaims() decodes with json_decode($raw, true), which erases the difference between a JSON array and a JSON object, so `"aud": {"target": "keepiq"}` arrived as a PHP array whose VALUES were iterated and matched. The strict typing added earlier in this PR rejected `123` and mixed arrays and then accepted an object - the same authenticate-on-the-well-formed-remainder hole, one shape further out. array_is_list() would not have fixed it, as Wilco noted: `{"0": "keepiq"}` decodes to a list. The distinction has to survive decoding, so the payload is now decoded WITHOUT assoc and cast at the top level only. Nested objects stay objects and are rejected; nested arrays stay arrays and are accepted. DEPRECATION WAS REPORTED BEFORE AUTHENTICATION. The audience check runs before replay detection, issuer lookup and signature verification - correctly, since it should reject early - but it also emitted the migration warning there. So a forged or replayed assertion produced the same "issuer X still needs to migrate" line as a successful exchange, naming an issuer nobody had verified. That log is the checklist for deciding when `doriath` can stop being accepted, so anyone could have filled it with issuers of their choosing and kept the shim alive indefinitely. Validation and reporting are now separate: assertNamesThisInstance() throws or returns whether the deprecated value was used, and JwtAuthService reports only after the signature, issuer and replay checks have all passed. Covered by the exchange-level tests Wilco asked for - both object shapes, plus a replayed and a badly signed assertion carrying `aud: doriath` asserting no warning is emitted. Verified by reverting each fix in turn: the object cases fail with the assoc decode restored, and both no-report cases fail with the warning moved back before verification. 1195 tests pass; phpmd, phpstan and phpcs clean. Assisted-by: ClaudeCode:claude-opus-5

github-actions · 2026-09-08T14:57:56Z

Quality Report — ConductionNL/keepiq @ `211feda`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 111/111
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	✅
PHPUnit					✅
Newman					✅
Playwright					✅
Hydra gates					✅

Quality workflow — 2026-09-08 14:57 UTC

Download the full PDF report from the workflow artifacts.

rjzondervan added 2 commits September 8, 2026 12:20

bbrands02 reviewed Sep 8, 2026

View reviewed changes

Comment thread lib/Service/AudiencePolicy.php Outdated

WilcoLouwerse reviewed Sep 8, 2026

View reviewed changes

Comment thread lib/AppInfo/Application.php

Comment thread lib/Controller/DiscoveryController.php

Comment thread lib/Controller/DiscoveryController.php

Comment thread lib/Service/JwtAssertionVerifier.php

Comment thread openspec/specs/secret-store-api/spec.md Outdated

WilcoLouwerse requested changes Sep 8, 2026

View reviewed changes

WilcoLouwerse approved these changes Sep 8, 2026

View reviewed changes

WilcoLouwerse previously approved these changes Sep 8, 2026

View reviewed changes

Comment thread lib/Service/JwtAuthService.php Outdated

Comment thread lib/Service/AudiencePolicy.php

rjzondervan dismissed WilcoLouwerse’s stale review via f10b8a8 September 8, 2026 14:39

WilcoLouwerse approved these changes Sep 8, 2026

View reviewed changes

rjzondervan merged commit 869afae into development Sep 9, 2026
49 checks passed

Navigation Menu

Uh oh!

feat(api): accept both keepiq and doriath on the machine API wire contract - #655

feat(api): accept both keepiq and doriath on the machine API wire contract#655
rjzondervan merged 7 commits into
developmentfrom
feat/pre-stable-wire-deprecations-v2

rjzondervan commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WilcoLouwerse left a comment

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

WilcoLouwerse left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rjzondervan commented Sep 8, 2026

Audience — dual-accept (inbound)

Discovery path — dual-serve (inbound)

Envelope format — announce only (outbound)

Deadline

Verification

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ f152b5a

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ 7ee187b

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ 5cad9ba

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ 4daadb7

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WilcoLouwerse left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ 21863f5

Uh oh!

WilcoLouwerse left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026

Quality Report — ConductionNL/keepiq @ 211feda

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Quality Report — ConductionNL/keepiq @ `f152b5a`

Quality Report — ConductionNL/keepiq @ `7ee187b`

Quality Report — ConductionNL/keepiq @ `5cad9ba`

Quality Report — ConductionNL/keepiq @ `4daadb7`

Quality Report — ConductionNL/keepiq @ `21863f5`

WilcoLouwerse left a comment •

edited

Loading

Quality Report — ConductionNL/keepiq @ `211feda`