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..b0c9a05dd 100644 --- a/src/cls/IPM/Repo/Oras/PackageService.cls +++ b/src/cls/IPM/Repo/Oras/PackageService.cls @@ -19,15 +19,132 @@ 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 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 { 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). +/// 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/")) { + return 0 + } + set statusCode = request.HttpResponse.StatusCode + if (200 <= statusCode) && (statusCode < 300) { + return 1 + } + 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 ..ProbeTokenEndpoint(wwwAuth) +} + +/// 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: 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 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 + } + set realm = realmMatcher.Group(1) + set serviceMatcher = ##class(%Regex.Matcher).%New("service=""([^""]+)""", wwwAuth) + set service = "" + 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 + } + 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) + } + // 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 + } elseif ..TokenAuthMethod = "basic" { + do tokenRequest.SetHeader("Authorization", "Basic " _ ..Token) + } + set path = $get(comp("path"), "") + if $extract(path, 1) = "/" { + set path = $extract(path, 2, *) + } + // 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) } Method GetModule( @@ -262,9 +379,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..6efe5ee7f 100644 --- a/tests/unit_tests/Test/PM/Unit/Oras.cls +++ b/tests/unit_tests/Test/PM/Unit/Oras.cls @@ -1,78 +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 as default registry - do ..RunCommand("repo -publish 1 -n oci") - - // Install from default - do ..AssertNoException("install " _ Name) + 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") +} - // Clean up - do ..RunCommand("repo -delete-all") - do ..RunCommand("repo -reset-defaults") +/// 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") } -Method RunCommand(pCommand As %String) +/// URL with a path component should set PathPrefix and strip the path from Location. +Method TestGetPackageServiceWithPathPrefix() { - do ##class(%IPM.Main).Shell(pCommand) - do $$$LogMessage("Run command: "_pCommand) + 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") } -Method AssertNoException(pCommand As %String) +/// Bare hostname with port should preserve port in Location. +Method TestGetPackageServiceUrlNormalizationWithPort() { - do ##class(%IPM.Main).ShellInternal(pCommand,.tException) - if '$$$AssertEquals(tException,"","No exceptions occurred running command: "_pCommand) { - do $$$LogMessage(tException.DisplayString()) - } + 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") } }