Repository navigation
feat(ocl): canonical resolution via $resolveReference (ConceptMap/ValueSet sources, released-version default) - #282
Open
italomacedo wants to merge 11 commits into
Open
italomacedo wants to merge 11 commits into
italomacedo wants to merge 11 commits into
Conversation
Client for OCL's $resolveReference: given a canonical URL (or relative OCL path), OCL answers which repo holds it. This replaces finding a repo by iterating source/collection listings and matching canonical_url client-side. Namespace is deliberately not supported. FHIR operations carry no namespace parameter, and a canonical URL is the same artifact regardless of which OCL namespace stores it -- so every resolution runs in OCL's global namespace. Namespace semantics (multi-tenancy, sandboxing) are an open discussion with the OCL team, not something to encode client-side yet. Behaviour notes, all verified against a live OCL instance: - $resolveReference is authenticated everywhere we probed, while the listing endpoints it replaces are public. Without a token the resolver is constructed disabled and callers keep their existing paths; 404/401/403 disable it for the rest of the process. - Batches go as one POST with the whole array; results are positional, so a count mismatch discards the batch rather than risk attributing a resolution to the wrong canonical. - The result carries the repo's own canonical_url and owner_type (the docs' example omits them); these are surfaced rather than echoing the request back. - url_registry_entry is surfaced even though every observed response carries null -- whether the URL Registry participates is exactly what the OCL-team discussion needs. The response-shape tests are built from a captured live payload, not from the documented example (which reports "Source Version" where OCL returns "Source"). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#candidateSourceUrls answered "which OCL repo holds this canonical?" by
heuristic: derive a search token from the URL, page through
/orgs/{org}/sources/?q=..., and match canonical_url on whatever the search
surfaces. Ask $resolveReference first; any failure falls back to that search, so
behaviour without a token is unchanged.
Two prerequisites are included because the resolver exposed them:
- searchConceptMaps lower-cased every param value. Every consumer compared
through #norm() (which lower-cases anyway) so nothing noticed, and the text
search tolerated it -- but $resolveReference matches the canonical exactly, so
every lookup silently failed to resolve until this was fixed.
- Candidate repo paths were filtered with startsWith('/orgs/'), silently dropping
user-owned repos (/users/{user}/...), which the resolver legitimately returns.
The repo's own canonical_url from the resolve result is recorded in the
canonical<->repo bookkeeping instead of echoing the caller's spelling.
Which path served a lookup is logged either way -- resolver and search return
the same thing, so success was otherwise unobservable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lveReference Covers the ValueSet provider's two canonical->repo points: - #findCollectionByCanonical: fetchValueSet for a canonical the enumeration did not bring in searched every org's /collections/ with a q= text token, matching canonical_url client-side. Ask $resolveReference first; a result that is not a collection, a version mismatch, or any failure falls through to the search. - compose sources: #buildCollectionSources resolved each source's canonical with one sequential GET per source. #primeSourceCanonicalsBatch resolves all of them in a single $resolveReference POST and seeds sourceCanonicalCache; anything unresolved falls back to the per-source GET, so this only saves round trips, never changes results. Both are no-ops without a token, matching the ConceptMap behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Policy decision: an artifact is expected to live in an organization to be
visible through the terminology service. User-owned repos (/users/{user}/...)
are experimental by convention and are excluded from resolution -- a canonical
that $resolveReference resolves to a user-owned repo is treated as unresolved
(logged, cached, callers fall back to their search paths).
isOclRepoPath therefore accepts only /orgs/ paths, and isOrgOwned checks the
explicit owner_type when a payload carries one (falling back to the path shape).
This supersedes the earlier reading that dropping /users/ repos in ConceptMap
lookups was a bug: it was the intended visibility rule, now stated and tested
rather than incidental.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Discovery enumerated /orgs/ and then listed /orgs/{org}/sources/ (and
/collections/) for every org -- N+1 listing requests, repeated on every refresh
cycle. The global /sources/ and /collections/ listings return the same set in
one paginated crawl; verified live: 15 orgs, per-org discovery fetched 417
sources, the global listing reports num_found=417 and boot now logs the same
"Fetched 417 sources" through a single crawl.
Entries are filtered with isOrgOwned() to honour the org-only visibility policy
(the global listing includes user-owned repos, which per-org enumeration never
saw -- live boot showed 463 collections globally, 450 kept after the filter).
The per-org path remains as fallback for instances where the global listing is
unavailable or empty.
Enumeration itself stays: a terminology server's catalog is built by discovery,
not by request traffic. This changes how the catalog is listed, not whether.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cept
searchConceptMaps has no concept code -- it asks "what mappings does this source
have?" -- but answered that with the per-concept endpoint: list the concepts,
then issue one request per concept and union the results.
Two problems, measured against the live OCL instance:
- Correctness. The concept listing is capped at maxSearchPages (10 x 100 =
1000). LOINC has 184,683 concepts, so it only ever saw 0.5% of them, and any
mapping on a concept past the first 1000 was silently invisible. No error --
just fewer results.
- Cost. Up to 1000 sequential requests per source. ConceptMap searches on loinc
and snomed timed out (45s+) and returned nothing at all; in production the
same shows up as 504s.
{source}/mappings/ answers the actual question in one paginated call. Verified
equivalent before switching: for a source with 2 mappings both paths return the
identical set, one in 1 request instead of 4. After the change, live:
http://loinc.org timeout(45s+), 0 results -> 200, 1 ConceptMap
http://snomed.info/sct timeout(45s+), 0 results -> 200, 5 ConceptMaps
The per-concept endpoint stays where it belongs: findConceptMapForTranslation
has a sourceCode and asks about that one concept, so its single targeted request
is already the right call. Untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After collecting a source's mappings, #ensureCanonicalForSourceUrls translated each from/to_source_url into its canonical with one sequential GET per source repo. $resolveReference answers the same question -- the result carries the repo's canonical_url -- for the whole set in a single POST. The per-source GET loop stays as the fallback for whatever the batch could not resolve (no token, endpoint unavailable, individual misses), so behaviour without a token is unchanged. This was the last canonical<->repo translation in tx/ocl still done by per-item requests; with it, every such lookup goes through $resolveReference when a token is configured: canonical -> repo ConceptMap source-system, ValueSet by canonical repo -> canonical ValueSet compose sources (batch), mapping sources (batch) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OCL's own resolution says a source's default version is its latest RELEASE (HEAD only when nothing is released) -- measured live: $resolveReference for http://loinc.org with no version answers 2.82, type "Source Version". But discovery listings only report HEAD, so FHIRsmith registered HEAD-only: versionless requests served the DRAFT, and requests for the released version got "unknown". A/B against main confirmed both pre-existing. With a token, discovery now batch-resolves every canonical and, where the default differs from HEAD, rewrites the snapshot entry to the release -- CodeSystem resource, meta and a version-scoped concepts URL (/{version}/concepts/, verified live) -- keeping the HEAD meta as an extra variant. getSourceMetas() returns defaults first, so registerProvider's first-wins unversioned key makes the release the versionless answer, while |HEAD and |{release} both resolve explicitly. Only new-or-changed canonicals are re-resolved on the minute refresh, so a quiet cycle costs no extra requests; a release being published or deleted flips the entry checksum, surfaces as "changed", and the refresh creates factories for versions that appeared. Without a token, discovery stays HEAD-only exactly as before. Two supporting changes ride along because the feature depends on them: - resolveReferences() now chunks batches at 100 references (the live instance 403s somewhere past 150) and takes a bypassCache option so refresh sees release changes; the cache is refreshed, not invalidated. - OCLSourceCodeSystemFactory registered itself under a SHA-256 of `system|version` while hasExactFactory/#findFactory look up the PLAIN string, so exact-version matching could never succeed -- only the unversioned `system|` alias worked. The key is in-memory only (maps, job keys, logs); it is now the plain string. Found the moment version-aware factory creation needed hasExactFactory to actually work. Live, on cmed (release 20230109): before: no version -> 200 version=HEAD ; version=20230109 -> 422 unknown after: no version -> 200 version=20230109 ; |20230109 -> 200 ; |HEAD -> 200 Boot reports the blast radius on this instance: [OCL] 176 code system(s) defaulting to a released version (HEAD kept as |HEAD) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s tests An audit of "do we have tests for everything implemented?" found four behaviours proven only live (or not at all). Now unit-tested: - Global-listing discovery (cs + vs): one crawl of /sources/ / /collections/, user-owned entries filtered by the org-only policy, and the per-org enumeration exercised as the fallback when the global listing fails. - ValueSet compose source canonicals resolved in ONE $resolveReference batch (no per-source detail GETs), seeding sourceCanonicalCache, with the compose built from the returned canonical_urls. - Version mismatch on a resolved collection falls back to the search instead of serving the wrong version. - Default-version steady state: a refresh over an unchanged listing issues NO new $resolveReference calls and keeps the release/HEAD registration intact. One test-authoring note: vs-ocl normalizes conceptsUrl to an absolute URL without a trailing slash, so mocks must match by substring — an exact-path mock silently returns empty and the batch never runs, which is how the first version of the compose test failed. Still live-only (documented, not unit-tested): post-boot factory creation for a release published after startup depends on the OCLSourceCodeSystemFactory sharedI18n singleton, which unit tests here cannot set up cheaply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…logger The $resolveReference / canonical-resolution work was authored before the module-logger convention landed (PR #266), so it used console.log/warn with [OCL]/[OCL-ValueSet] prefixes. Convert those 14 calls to child loggers to keep the module consistent, per the same review feedback addressed in #266: - cm-ocl.cjs: new child logger { module: 'ocl-cm' } (5 calls) - cs-ocl.cjs: existing oclLog (3 calls) - vs-ocl.cjs: existing oclVsLog (6 calls) Redundant [OCL] message prefixes dropped (the child logger tags each line with {module}). No behavior change beyond log routing; all 165 OCL tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Claude review: Security The OCL token is shared with every anonymous caller. The PR effectively needs token= configured: without it the resolver is off and nothing new happens. The HTTP client puts that token on every OCL request, and the resolver answers canonicals from any request ($expand?url=…, ConceptMap source-system). So anything the token's account can see can be reached through the public tx server. Minor: the resolver is constructed without a logger, so it logs to console rather than the module logger the description claims. Those log lines include the client-supplied URL unescaped. |
Security/robustness fixes from the PR #282 review (Grahame). All within tx/ocl/**. #2 repoUrl validation / SSRF surface: - normalizeResult now rejects any repoUrl that is not a safe same-host relative path (new isSafeRelativeOclPath: must start with '/', not '//', no '..'), so a surprising OCL response can never redirect an authed request (carrying our token) off-host. - isOrgOwned no longer trusts owner_type alone: it requires a valid org repo path AND (when present) owner_type === 'Organization'. Closes the hole where {owner_type:'Organization'} on a non-org/off-host url was accepted. - REPO_PATH_PATTERN tightened to /^\/orgs\/[^/]+\/(sources|collections)\/[^/]+\// and isOclRepoPath rejects any '..' (path traversal). #3 bounded cache + input caps: - the resolution cache is now a bounded LRU (DEFAULT_CACHE_LIMIT, evicts oldest), so client-supplied references can no longer grow memory without bound. - references longer than MAX_REFERENCE_LENGTH are treated as unresolved and never sent to OCL. #4 transient-failure back-off: - a 401/403 now backs the resolver off for a cooldown (DEFAULT_AUTH_BACKOFF_MS) instead of disabling it for the whole process; 404 stays permanently disabled. #5 logging: - the resolver is given the module child logger by cm-ocl/vs-ocl (was defaulting to console); [OCL] prefixes dropped (the child logger tags {module}); all client-supplied values in log lines are sanitised (safeForLog: strips control chars, caps length) to prevent log forging. Note: isOrgOwned tests updated to assert the hardened behavior (owner_type alone no longer grants access). 173 OCL tests pass. Finding #1 (public_access gate) is held pending a live $resolveReference payload capture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
Adds
$resolveReference-based canonical resolution to the OCL provider, so ConceptMap sources and ValueSet collections/compose sources are resolved by their global canonical URL rather than by guessing repo paths or walking concepts. Validated in production (tx.gointerop.com / Brazil DATASUS instance) since 2026-07-15 with no follow-up fixes. Scoped entirely totx/ocl/**+tests/ocl/**.This supersedes the earlier, incomplete #267.
What's included
$resolveReferenceclient (tx/ocl/resolve/reference-resolver.js) — resolves a canonical URL to its OCL repo via the global namespace; chunked at 100 refs/request (the instance 403s past ~150); no-op with a clear disabled reason when no token is configured.cm-ocl.cjsresolves source canonicals via$resolveReference, with a single batch call for the mixed set and a fallback to source search; fetches{source}/mappings/directly instead of walking every concept.vs-ocl.cjsresolves collections and compose sources the same way, batched.|HEADremains addressable as an explicit variant (factories created for bothmetaandheadMeta)./sources/,/collections/) with per-org fallback.Logger.getInstance().child({ module })), consistent with fix(ocl): auto-register factories for post-startup sources + expose concept extras in $lookup #266.Requirements / notes
token=on theocl:source line (kept in the deployment'sdata/library.ymlvolume; never committed). Without a token the resolver is disabled and the provider falls back to source search.Tests
5 new
tests/ocl/*suites (resolver, CM/VS resolution, default-version, global discovery). Full OCL suite: 165 tests passing on the currentmainbase.🤖 Generated with Claude Code