🚀 Auto-PR: Merge test → main - #1022
Merged
Merged
Conversation
A link scoped to one imaging study told the recipient Complete summary — the patient shared their full health record while the proxy answered ~27 of the viewer's 30 queries with 403 "outside shared study scope". The viewer drew each denial as its empty state, so the screen read "No known allergies", "No active medications", "No active conditions" for a record nobody was allowed to see. Denied and absent are not the same claim, and this one is the dangerous direction. `complete` was computed inline at the mint site as `!shareScope`, which knew about selective de-selection because that is what it shipped with, and never learned about study scoping — even though study scoping narrows harder (default-deny to this patient, this study, metadata). Extracted to isCompleteShare beside the other scope rules, so the next narrowing dimension has one obvious place to be counted, with tests including the case that regressed. Verified against the live beta link: 27 of 30 queries 403, only Patient, ImagingStudy and metadata pass. The viewer half — dropping cards that have nothing to show, so a denial cannot be read as an assertion — is in patient-portal.
🧪 Auto-PR: Merge `develop` → `test`
Two independent faults, both fatal to exactly the audience a system-scoped FHIR API has, and both invisible on beta because beta runs behind Caddy with no WAF. The WAF blocked every request with no User-Agent header. Server-side HTTP clients do not send one unless told to — Cloudflare Workers' fetch is one, anything on undici defaults is another. Measured on production: the same URL answers 200 with `-A curl/8.0` and 403 with `-A ""`, for both .well-known/smart-configuration and /metadata. So SMART discovery failed for every conformant headless client. NoUserAgent_HEADER joins the excluded sub-rules, which sets it to COUNT: it still evaluates and still reports, it just stops terminating the request. A bot heuristic that blocks machine clients is wrong for this API specifically. The CapabilityStatement advertised no OAuth endpoints. HAPI does not know it sits behind a SMART authorization layer, so it returns rest[].security empty, and /metadata was passed through with URL rewriting only. We are that layer, so withSmartSecurity adds the oauth-uris extension and the SMART-on-FHIR service coding from the same service that builds .well-known/smart-configuration — the two documents cannot disagree. A server that already declares its own endpoints keeps them, a body that is not a CapabilityStatement is passed through untouched, and a discovery failure serves the upstream document rather than an error. Found from the other side: maxhealth.tech's provisioning job reported "Failed to discover SMART endpoints" on every run and a member's record went unwritten, while curl against the same URL returned 200.
…mint The completeness fix in 9c9b19b could not reach a single link already in circulation, and this is why: encryptSHLFile runs at mint, the ciphertext is stored on the session, and the manifest endpoint replays it verbatim (`embedded: entry.jwe`). Whatever the document claimed on the day it was created, it claims forever. A link minted before that fix still tells its recipient Complete summary — the patient shared their full health record over a view where the proxy denies all but Patient, ImagingStudy and metadata. Nothing required the snapshot: the session already holds studyInstanceUID, shareScope, patientId, sessionToken, expiresAt and the SHL key, so the manifest can state what is true when asked. It now rebuilds and re-encrypts the document on each fetch, falling back to the stored blob only if encryption fails — a stale claim still beats handing the recipient a link that will not open. Mint and manifest share one buildSmartApiAccess so a fresh link and a later fetch cannot disagree. `expires_in` is now counted from the request rather than from mint, where it had been describing a lifetime that started days ago. Existing links, including the one that prompted this, are corrected as soon as the backend deploys. No re-share needed.
🧪 Auto-PR: Merge `develop` → `test`
Keycloak verifies proxy-signed assertions by fetching this backend's JWKS
through the proxy-smart-signing IdP. `ensureProxySigningIdp` derived that
URL as `http://backend:${port}` whenever Keycloak's host was not loopback
— a docker-compose service name. On ECS nothing answers to `backend`, so
Keycloak can verify nothing and every private_key_jwt client fails with
`invalid_client`.
Production held `http://localhost:8445` for issuer, tokenUrl and jwksUrl,
which is not what the current code writes — so those are stale values the
reconciler never corrected. It was fixed by hand to the public URLs, and
the next deploy would have replaced that fix with `http://backend:8445`,
which is equally unreachable there. That is the trap this closes.
PROXY_SIGNING_JWKS_URL now names the URL, because only the deployment
knows how its network is wired; the backend stack sets it to the public
base URL, which Keycloak reaches over its existing egress — proven, since
that is the value that made prod issue tokens at all. Unset, the
docker-compose default is unchanged, which is what beta runs on.
And the reconcile refuses outright when the URL is loopback while Keycloak
is elsewhere. That combination is always wrong and always silent, so
leaving a working config alone beats replacing it with a broken one. A
compose service name still passes: unresolvable from here, resolvable from
Keycloak, and not ours to second-guess.
The rules live in their own module so they are tested directly rather than
mirrored — importing init starts a server.
🧪 Auto-PR: Merge `develop` → `test`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Pull Request 🤖
This PR was automatically created to merge changes from
testintomain.Changes:
Review: Please review the changes before merging.
Last updated: 2026-08-14 20:43:13 UTC