From e5dc871eb9fdab08699d2e5e297879982d717f4f Mon Sep 17 00:00:00 2001 From: isc-dchui Date: Mon, 4 May 2026 16:35:16 -0400 Subject: [PATCH 1/4] Fix ORAS auth flow --- CHANGELOG.md | 1 + src/cls/IPM/DataType/RepoLocation.cls | 4 +- src/cls/IPM/Repo/Oras/Definition.cls | 18 +++- src/cls/IPM/Repo/Oras/PackageService.cls | 115 ++++++++++++++++++++++- tests/unit_tests/Test/PM/Unit/Oras.cls | 89 +++++++----------- 5 files changed, 165 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fcf45db1..e3aa4b60b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - #1110: Add `iriscli` and `ipm` container utility scripts that are auto-installed to `~/.local/bin/` and `~/bin/` so they work both inside and outside of containers (Unix/Linux only) ### Fixed +- #1130: Fix issue with ORAS repositories pointing to some OCI registries that require authentication (e.g. ghcr.io) not accepting credentials properly. `repo -list` now shows an `Authenticated?` status for ORAS repos with credentials configured. - #1001: The `unmap` and `enable` commands will now only activate CPF merge once after all namespaces have been configured instead after every namespace - #1052: In a namespace with mapped IPM, the `info` command works again and the intro message displays the IPM version and where its mapped from - #1102: %IPM.Storage.QualifiedModuleInfo:%New() will now copy over version properties when passed in a resolvedReference diff --git a/src/cls/IPM/DataType/RepoLocation.cls b/src/cls/IPM/DataType/RepoLocation.cls index 2f4e5084c..fa95ee50e 100644 --- a/src/cls/IPM/DataType/RepoLocation.cls +++ b/src/cls/IPM/DataType/RepoLocation.cls @@ -3,6 +3,8 @@ Class %IPM.DataType.RepoLocation Extends %IPM.DataType.RegExString [ ClassType = Parameter MAXLEN = 2048; -Parameter REGEX = "https?:\/\/(?:[a-zA-Z0-9-]+\.)*[a-zA-Z0-9-]+(?::\d+)?(?:\/[^\s]*)?\/?"; +// Scheme is optional: OCI/ORAS convention uses bare hostnames (e.g. ghcr.io, not https://ghcr.io). +// https:// is assumed when no scheme is present. +Parameter REGEX = "(https?:\/\/)?(?:[a-zA-Z0-9-]+\.)*[a-zA-Z0-9-]+(?::\d+)?(?:\/[^\s]*)?\/?"; } diff --git a/src/cls/IPM/Repo/Oras/Definition.cls b/src/cls/IPM/Repo/Oras/Definition.cls index 629e15831..0c420d8b0 100644 --- a/src/cls/IPM/Repo/Oras/Definition.cls +++ b/src/cls/IPM/Repo/Oras/Definition.cls @@ -27,8 +27,17 @@ Method GetPackageService() As %IPM.Repo.IPackageService set tClient = ##class(%IPM.Repo.Oras.PackageService).%New() // We want to preprend the prefix to the "GET"/"POST"/etc. requests // Otherwise, it will be overwritten - do ##class(%Net.URLParser).Decompose(..URL,.comp) - if $data(comp("path"), path) && (path '= "") { + set tUrl = ..URL + // %Net.URLParser.Decompose misparses bare hostnames as path components; scheme must be present. + if ($piece(tUrl, "://")'["http") { + set tUrl = "https://" _ tUrl + } + do ##class(%Net.URLParser).Decompose(tUrl,.comp) + if $data(comp("path"), path) && (path '= "") && (path '= "/") { + // Decompose includes a leading slash; strip it for use as a prefix + if $extract(path, 1) = "/" { + set path = $extract(path, 2, *) + } if $extract(path, *) '= "/" { set path = path _ "/" } @@ -65,6 +74,11 @@ Method Display() if (..Namespace '= "") { write !,$char(9),"Namespace: ",..Padding(1),..Namespace } + if (..Username '= "") || (..Password '= "") || (..Token '= "") { + set packageService = ..GetPackageService() + set authenticated = (packageService.IsAvailable() && packageService.IsAuthenticated()) + write !,$char(9),"Authenticated?",..Padding(1),$$$YesNo(authenticated) + } } /// Handles modifiers/data attributes provided from the package manager shell. diff --git a/src/cls/IPM/Repo/Oras/PackageService.cls b/src/cls/IPM/Repo/Oras/PackageService.cls index e03b7d84f..8e1c537c7 100644 --- a/src/cls/IPM/Repo/Oras/PackageService.cls +++ b/src/cls/IPM/Repo/Oras/PackageService.cls @@ -19,15 +19,117 @@ Parameter ALLOWPREFIXINLOCATION = 0; // ** PACKAGE SERVICE FUNCTIONS ** /// For run-time checks to ensure the service is available before attempting to invoke it. +/// Returns true if the registry is reachable. +/// Per the Docker Distribution Spec, GET /v2/ returning 401 means the registry is up but requires +/// authentication — this is not a failure. We check for WWW-Authenticate (required by RFC 7235 on +/// any proper 401) rather than Docker-Distribution-Api-Version, since many registries omit the latter. Method IsAvailable() As %Boolean { set request = ..GetHttpRequest(..Location) set request.Timeout = 5 if $$$ISERR(request.Get(..PathPrefix _ "v2/")) { - quit 0 + return 0 } set statusCode = request.HttpResponse.StatusCode - quit (200 <= statusCode) && (statusCode < 300) + if (200 <= statusCode) && (statusCode < 300) { + return 1 + } + if statusCode = 401 { + return (request.HttpResponse.GetHeader("WWW-AUTHENTICATE") '= "") + } + return 0 +} + +/// Returns true if the configured credentials are accepted by the registry. +/// Always returns true when no credentials are configured (anonymous access). +Method IsAuthenticated() As %Boolean +{ + if (..Username = "") && (..Password = "") && (..Token = "") { + return 1 + } + set request = ..GetHttpRequest(..Location) + set request.Timeout = 5 + if $$$ISERR(request.Get(..PathPrefix _ "v2/")) { + return 0 + } + set statusCode = request.HttpResponse.StatusCode + if (200 <= statusCode) && (statusCode < 300) { + return 1 + } + if statusCode '= 401 { + return 0 + } + set wwwAuth = request.HttpResponse.GetHeader("WWW-AUTHENTICATE") + return ..DoBearerTokenExchange(wwwAuth) +} + +/// Performs the Docker Distribution Spec Bearer token exchange to validate credentials. +/// We do this in ObjectScript rather than delegating to the Python ORAS client because +/// oras-py has no request timeout — client.get_tags() can hang indefinitely/for a long time. +/// Returns true if the token endpoint accepts the credentials. +Method DoBearerTokenExchange(wwwAuth As %String) As %Boolean [ Private ] +{ + if wwwAuth = "" { + return 0 + } + set scheme = $zconvert($piece(wwwAuth, " ", 1), "L") + // Basic challenge: credentials were already sent preemptively by GetHttpRequest() and rejected + if scheme = "basic" { + return 0 + } + if scheme '= "bearer" { + return 0 + } + // Bearer/apiKey tokens were already sent in headers by GetHttpRequest() and rejected + if (..Token '= "") && ((..TokenAuthMethod = "bearer") || (..TokenAuthMethod = "apiKey")) { + return 0 + } + set realmMatcher = ##class(%Regex.Matcher).%New("realm=""([^""]+)""", wwwAuth) + if 'realmMatcher.Locate() { + return 0 + } + set realm = realmMatcher.Group(1) + set serviceMatcher = ##class(%Regex.Matcher).%New("service=""([^""]+)""", wwwAuth) + set service = "" + if serviceMatcher.Locate() { + set service = serviceMatcher.Group(1) + } + set tokenUrl = realm + if ($piece(tokenUrl, "://") '[ "http") { + set tokenUrl = "https://" _ tokenUrl + } + do ##class(%Net.URLParser).Decompose(tokenUrl, .comp) + set tokenRequest = ##class(%Net.HttpRequest).%New() + set tokenRequest.Timeout = 5 + set tokenRequest.FollowRedirect = 1 + set tokenRequest.Server = comp("host") + if ($data(comp("port")) # 2) && (comp("port") '= "") { + set tokenRequest.Port = comp("port") + } + if $zconvert(comp("scheme"), "L") = "https" { + set tokenRequest.Https = 1 + set tokenRequest.SSLConfiguration = ..GetSSLConfiguration(tokenRequest.Server) + } + if (..Username '= "") && (..Password '= "") { + set tokenRequest.Username = ..Username + set tokenRequest.Password = ..Password + } elseif ..TokenAuthMethod = "basic" { + do tokenRequest.SetHeader("Authorization", "Basic " _ ..Token) + } + set path = $get(comp("path"), "") + if $extract(path, 1) = "/" { + set path = $extract(path, 2, *) + } + // scope omitted — token endpoint is used only as a credential probe, not to request + // repository access. Registries that require scope may return 400, causing this probe + // to report unauthenticated even when credentials are valid. + if service '= "" { + set path = path _ "?service=" _ service + } + if $$$ISERR(tokenRequest.Get(path)) { + return 0 + } + return (200 <= tokenRequest.HttpResponse.StatusCode) && (tokenRequest.HttpResponse.StatusCode < 300) } Method GetModule( @@ -262,9 +364,12 @@ Method ListModules(pSearchCriteria As %IPM.Repo.SearchCriteria) As %ListOfObject } do ..ListModulesFromTagString(tVersionExpression, client, pSearchCriteria, name, .tList) } elseif response.StatusCode '= 200 { - // If no name is specified and the /v2/_catalog endpoint failed, error out - set msg = "Error: Call to /v2/_catalog endpoint failed. This registry may not support it." - set msg = msg _ $char(10,13) _ "Response Code: "_response.StatusCode _ " - " _ response.ReasonPhrase + if response.StatusCode = 401 { + set msg = "Error: Registry requires authorization for catalog listing or does not support it." + } else { + set msg = "Error: Call to /v2/_catalog endpoint failed. This registry may not support it." + set msg = msg _ $char(10,13) _ "Response Code: "_response.StatusCode _ " - " _ response.ReasonPhrase + } $$$ThrowStatus($$$ERROR($$$GeneralError, msg)) } } diff --git a/tests/unit_tests/Test/PM/Unit/Oras.cls b/tests/unit_tests/Test/PM/Unit/Oras.cls index 3f49a194a..180256717 100644 --- a/tests/unit_tests/Test/PM/Unit/Oras.cls +++ b/tests/unit_tests/Test/PM/Unit/Oras.cls @@ -1,64 +1,45 @@ Class Test.PM.Unit.Oras Extends %UnitTest.TestCase { -Method TestOras() +/// Bare hostname (no scheme) should produce a Location with https:// scheme +/// and no path leaking into Location or PathPrefix. +Method TestGetPackageServiceUrlNormalizationBareHostname() { - set Registry = "http://oras:5000" - set Name = "objectscript-math" - - // Target package - set moduleRef = ##class(%IPM.Storage.ModuleInfo).%New() - set moduleRef.Name = Name - - // Cleanup - do ..RunCommand("repo -delete-all") - do ..RunCommand("repo -reset-defaults") - - // Set up ORAS repo - do ..AssertNoException("repo -o -name oci -url " _ Registry) - do ..RunCommand("repo -list") - do ..RunCommand("repo -list-modules") - - &sql(SELECT id INTO :ociId FROM %IPM_Repo_Oras.definition WHERE name='oci') - do $$$AssertEquals(SQLCODE, 0) - set oci = ##class(%IPM.Repo.Oras.Definition).%OpenId(ociId) - set packageService = oci.GetPackageService() - set publishService = oci.GetPublishService() - - if 'packageService.IsAvailable() { - do $$$AssertSkipped("ORAS registry unavailable, skipping") - } - - // make sure package is not in the registry first - set version = packageService.GetLatestTag(Registry, Name, "", "", "", "", 0) - if version '= "" { - do publishService.DeleteModule(Registry, Name, "", "", "", "", 0) - } - do $$$AssertNotTrue(packageService.HasModule(moduleRef)) - - // Install package from the default registry - do ..RunCommand("install registry/" _ Name) - - // Push to OCI registry - do ..AssertNoException("publish " _ Name _ " -r oci") - do $$$AssertTrue(packageService.HasModule(moduleRef)) - - // Pull from OCI registry - do ..RunCommand("uninstall " _ Name) - do ..AssertNoException("install oci/" _ Name) - - // Delete from OCI registry - do publishService.DeleteModule(Registry, Name, "", "", "", "", 0) + set def = ##class(%IPM.Repo.Oras.Definition).%New() + set def.URL = "ghcr.io" + set svc = def.GetPackageService() + do $$$AssertEquals(svc.Location, "https://ghcr.io", "Bare hostname gets https:// prepended") + do $$$AssertEquals(svc.PathPrefix, "", "No path prefix for bare hostname") +} - // Set as default registry - do ..RunCommand("repo -publish 1 -n oci") +/// Explicit https:// scheme should produce the same result as a bare hostname. +Method TestGetPackageServiceUrlNormalizationWithScheme() +{ + set def = ##class(%IPM.Repo.Oras.Definition).%New() + set def.URL = "https://ghcr.io" + set svc = def.GetPackageService() + do $$$AssertEquals(svc.Location, "https://ghcr.io", "Explicit https:// scheme preserved") + do $$$AssertEquals(svc.PathPrefix, "", "No path prefix when URL has no path") +} - // Install from default - do ..AssertNoException("install " _ Name) +/// URL with a path component should set PathPrefix and strip the path from Location. +Method TestGetPackageServiceWithPathPrefix() +{ + set def = ##class(%IPM.Repo.Oras.Definition).%New() + set def.URL = "ghcr.io/myorg" + set svc = def.GetPackageService() + do $$$AssertEquals(svc.Location, "https://ghcr.io", "Path stripped from Location") + do $$$AssertEquals(svc.PathPrefix, "myorg/", "Path component becomes PathPrefix") +} - // Clean up - do ..RunCommand("repo -delete-all") - do ..RunCommand("repo -reset-defaults") +/// Bare hostname with port should preserve port in Location. +Method TestGetPackageServiceUrlNormalizationWithPort() +{ + set def = ##class(%IPM.Repo.Oras.Definition).%New() + set def.URL = "registry.example.com:5000" + set svc = def.GetPackageService() + do $$$AssertEquals(svc.Location, "https://registry.example.com:5000", "Port preserved in Location") + do $$$AssertEquals(svc.PathPrefix, "", "No path prefix when no path in URL") } Method RunCommand(pCommand As %String) From 536bd3e1b9f4965c8eb80cc0135ee839698944f4 Mon Sep 17 00:00:00 2001 From: isc-dchui Date: Tue, 5 May 2026 10:18:05 -0400 Subject: [PATCH 2/4] Add some method documentation --- src/cls/IPM/Repo/Oras/PackageService.cls | 37 +++++++++++++++++------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/cls/IPM/Repo/Oras/PackageService.cls b/src/cls/IPM/Repo/Oras/PackageService.cls index 8e1c537c7..36ada2219 100644 --- a/src/cls/IPM/Repo/Oras/PackageService.cls +++ b/src/cls/IPM/Repo/Oras/PackageService.cls @@ -42,11 +42,16 @@ Method IsAvailable() As %Boolean /// Returns true if the configured credentials are accepted by the registry. /// Always returns true when no credentials are configured (anonymous access). +/// This is used only for display purposes (repo -list). Actual operations (pull, push, +/// list modules) delegate to the Python ORAS client via GetClient(), which handles the +/// full token exchange independently. Method IsAuthenticated() As %Boolean { if (..Username = "") && (..Password = "") && (..Token = "") { return 1 } + // Probe GET /v2/ with credentials attached by GetHttpRequest(). + // Some registries accept credentials directly here and return 2xx. set request = ..GetHttpRequest(..Location) set request.Timeout = 5 if $$$ISERR(request.Get(..PathPrefix _ "v2/")) { @@ -59,31 +64,37 @@ Method IsAuthenticated() As %Boolean if statusCode '= 401 { return 0 } + // 401 with a WWW-Authenticate header means the registry uses a token endpoint for auth. + // Parse the challenge and probe that endpoint to verify the credentials. set wwwAuth = request.HttpResponse.GetHeader("WWW-AUTHENTICATE") - return ..DoBearerTokenExchange(wwwAuth) + return ..ProbeTokenEndpoint(wwwAuth) } -/// Performs the Docker Distribution Spec Bearer token exchange to validate credentials. -/// We do this in ObjectScript rather than delegating to the Python ORAS client because -/// oras-py has no request timeout — client.get_tags() can hang indefinitely/for a long time. -/// Returns true if the token endpoint accepts the credentials. -Method DoBearerTokenExchange(wwwAuth As %String) As %Boolean [ Private ] +/// Probes the Bearer token endpoint from a WWW-Authenticate challenge to check whether +/// the configured credentials are accepted. We do not use the returned token — a 2xx +/// response is sufficient to confirm the credentials are valid. +/// We probe here rather than delegating to the Python ORAS client because oras-py has +/// no request timeout — client.get_tags() can hang indefinitely/for a long time. +Method ProbeTokenEndpoint(wwwAuth As %String) As %Boolean [ Private ] { if wwwAuth = "" { return 0 } set scheme = $zconvert($piece(wwwAuth, " ", 1), "L") - // Basic challenge: credentials were already sent preemptively by GetHttpRequest() and rejected + // Basic challenge: GetHttpRequest() already sent the credentials on /v2/ and they were rejected if scheme = "basic" { return 0 } if scheme '= "bearer" { return 0 } - // Bearer/apiKey tokens were already sent in headers by GetHttpRequest() and rejected + // bearer/apiKey tokens were already sent as headers by GetHttpRequest() on /v2/ and rejected — + // sending them again to the token endpoint won't help if (..Token '= "") && ((..TokenAuthMethod = "bearer") || (..TokenAuthMethod = "apiKey")) { return 0 } + // Parse the token endpoint URL (realm) and optional service identifier from the challenge header. + // Example: Bearer realm="https://ghcr.io/token",service="ghcr.io" set realmMatcher = ##class(%Regex.Matcher).%New("realm=""([^""]+)""", wwwAuth) if 'realmMatcher.Locate() { return 0 @@ -94,6 +105,7 @@ Method DoBearerTokenExchange(wwwAuth As %String) As %Boolean [ Private ] if serviceMatcher.Locate() { set service = serviceMatcher.Group(1) } + // Build a request to the token endpoint (which may be on a different host than the registry) set tokenUrl = realm if ($piece(tokenUrl, "://") '[ "http") { set tokenUrl = "https://" _ tokenUrl @@ -110,6 +122,8 @@ Method DoBearerTokenExchange(wwwAuth As %String) As %Boolean [ Private ] set tokenRequest.Https = 1 set tokenRequest.SSLConfiguration = ..GetSSLConfiguration(tokenRequest.Server) } + // Attach credentials: username+password via Basic auth, or a "basic"-method token as a + // pre-encoded Basic header (used by e.g. AWS ECR which issues a raw user:pass token string) if (..Username '= "") && (..Password '= "") { set tokenRequest.Username = ..Username set tokenRequest.Password = ..Password @@ -120,15 +134,16 @@ Method DoBearerTokenExchange(wwwAuth As %String) As %Boolean [ Private ] if $extract(path, 1) = "/" { set path = $extract(path, 2, *) } - // scope omitted — token endpoint is used only as a credential probe, not to request - // repository access. Registries that require scope may return 400, causing this probe - // to report unauthenticated even when credentials are valid. + // scope is omitted — we are only probing whether the credentials are accepted, not + // requesting access to a specific repository. Registries that strictly require scope + // may return 400, causing this probe to falsely report unauthenticated. if service '= "" { set path = path _ "?service=" _ service } if $$$ISERR(tokenRequest.Get(path)) { return 0 } + // 2xx means the token endpoint accepted the credentials — we don't need the token itself return (200 <= tokenRequest.HttpResponse.StatusCode) && (tokenRequest.HttpResponse.StatusCode < 300) } From b536281096c449698c426b162ca490e3a466908a Mon Sep 17 00:00:00 2001 From: isc-dchui Date: Tue, 5 May 2026 10:54:19 -0400 Subject: [PATCH 3/4] Fix slightly inaccurate comment --- src/cls/IPM/Repo/Oras/PackageService.cls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cls/IPM/Repo/Oras/PackageService.cls b/src/cls/IPM/Repo/Oras/PackageService.cls index 36ada2219..b0c9a05dd 100644 --- a/src/cls/IPM/Repo/Oras/PackageService.cls +++ b/src/cls/IPM/Repo/Oras/PackageService.cls @@ -20,7 +20,7 @@ Parameter ALLOWPREFIXINLOCATION = 0; /// For run-time checks to ensure the service is available before attempting to invoke it. /// Returns true if the registry is reachable. -/// Per the Docker Distribution Spec, GET /v2/ returning 401 means the registry is up but requires +/// Per the OCI Distribution Spec, GET /v2/ returning 401 means the registry is up but requires /// authentication — this is not a failure. We check for WWW-Authenticate (required by RFC 7235 on /// any proper 401) rather than Docker-Distribution-Api-Version, since many registries omit the latter. Method IsAvailable() As %Boolean From 98168c2f906334cbe219b01cf455cc347108f304 Mon Sep 17 00:00:00 2001 From: isc-dchui Date: Tue, 5 May 2026 11:10:36 -0400 Subject: [PATCH 4/4] Remove dead code --- tests/unit_tests/Test/PM/Unit/Oras.cls | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/unit_tests/Test/PM/Unit/Oras.cls b/tests/unit_tests/Test/PM/Unit/Oras.cls index 180256717..6efe5ee7f 100644 --- a/tests/unit_tests/Test/PM/Unit/Oras.cls +++ b/tests/unit_tests/Test/PM/Unit/Oras.cls @@ -42,18 +42,4 @@ Method TestGetPackageServiceUrlNormalizationWithPort() do $$$AssertEquals(svc.PathPrefix, "", "No path prefix when no path in URL") } -Method RunCommand(pCommand As %String) -{ - do ##class(%IPM.Main).Shell(pCommand) - do $$$LogMessage("Run command: "_pCommand) -} - -Method AssertNoException(pCommand As %String) -{ - do ##class(%IPM.Main).ShellInternal(pCommand,.tException) - if '$$$AssertEquals(tException,"","No exceptions occurred running command: "_pCommand) { - do $$$LogMessage(tException.DisplayString()) - } -} - }