From e96455579468022a40ff3f0585143e36bb4da995 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:03:08 +0300 Subject: [PATCH 1/3] feat(helm): read the registry a values document declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A values file usually splits the registry off from the repository: `repository: my-service` under a `global.imageRegistry` names `myreg.example.com/my-service`, and reading the repository alone gets the image wrong. Every reference now carries the registry its document declared, so the registry package has something to resolve a bare name against. A sibling `registry` key wins, then a document-level one found through a precedence-ordered table of key paths: `global.imageRegistry`, `global.registry`, `imageRegistry`, `registry`. A table rather than a chain of conditionals, because that list is the whole rule — the next convention somebody's charts follow is a new row, not a new branch. The declared string is emitted as written, from raw source text with a real range, exactly as `repository` and `tag` are. Deciding what a host means, and whether the string even is one, is OCI naming semantics and belongs in the registry package. "Templated, non-scalar, or empty counts as absent" was already spelled out twice and would have been four sites once registry joined, so it folds into one `readUsableScalar` that every field reads through. A templated sibling registry therefore falls through to the document-level one instead of shadowing it, which is what a chart writing `registry: "{{ .Values.global.registry }}"` means by it. The extension's test fixtures gain the new field. Nothing in the extension reads it yet; that arrives with the resolution chain. Refs #23 Co-Authored-By: Claude Opus 5 (1M context) --- apps/vscode/src/diagnostics.test.ts | 1 + apps/vscode/src/hover.test.ts | 2 + apps/vscode/src/marks.test.ts | 1 + .../helm/src/extract-image-references.test.ts | 111 ++++++++++++++++++ packages/helm/src/extract-image-references.ts | 82 +++++++++++-- 5 files changed, 185 insertions(+), 12 deletions(-) diff --git a/apps/vscode/src/diagnostics.test.ts b/apps/vscode/src/diagnostics.test.ts index 5a1c6d4..d199762 100644 --- a/apps/vscode/src/diagnostics.test.ts +++ b/apps/vscode/src/diagnostics.test.ts @@ -41,6 +41,7 @@ function createCheck(verdict: ReferenceVerdict): ReferenceCheck { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, tag: { text: TAG, range: rangeOfText(TAG) }, + registry: undefined, }, verdict, }; diff --git a/apps/vscode/src/hover.test.ts b/apps/vscode/src/hover.test.ts index 81cda4e..02ecd94 100644 --- a/apps/vscode/src/hover.test.ts +++ b/apps/vscode/src/hover.test.ts @@ -41,6 +41,7 @@ function createCheck(verdict: ReferenceVerdict): ReferenceCheck { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, tag: { text: TAG, range: rangeOfText(TAG) }, + registry: undefined, }, verdict, }; @@ -52,6 +53,7 @@ function createTaglessCheck(verdict: ReferenceVerdict): ReferenceCheck { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, tag: undefined, + registry: undefined, }, verdict, }; diff --git a/apps/vscode/src/marks.test.ts b/apps/vscode/src/marks.test.ts index 6ece276..a8c9a8c 100644 --- a/apps/vscode/src/marks.test.ts +++ b/apps/vscode/src/marks.test.ts @@ -27,6 +27,7 @@ function createCheck(verdict: ReferenceVerdict): ReferenceCheck { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, tag: { text: TAG, range: rangeOfText(TAG) }, + registry: undefined, }, verdict, }; diff --git a/packages/helm/src/extract-image-references.test.ts b/packages/helm/src/extract-image-references.test.ts index 17eafa1..42fb404 100644 --- a/packages/helm/src/extract-image-references.test.ts +++ b/packages/helm/src/extract-image-references.test.ts @@ -172,6 +172,117 @@ describe('extractImageReferences', () => { }); }); + describe('registry', () => { + it('should report a sibling registry key as the reference registry', () => { + const source = ['image:', ' repository: my-service', ' registry: sibling.example.com', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('sibling.example.com'); + }); + + it('should apply a top-level registry key to a reference with no sibling registry', () => { + const source = ['registry: top.example.com', 'image:', ' repository: my-service', ' tag: 1.0', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('top.example.com'); + }); + + it('should prefer global.imageRegistry over a top-level registry', () => { + const source = ['global:', ' imageRegistry: global.example.com', 'registry: top.example.com', 'image:', ' repository: my-service', ''].join( + '\n' + ); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('global.example.com'); + }); + + it('should apply global.registry when global.imageRegistry is absent', () => { + const source = ['global:', ' registry: global.example.com', 'image:', ' repository: my-service', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('global.example.com'); + }); + + it('should prefer a sibling registry over a document-level one', () => { + const source = [ + 'global:', + ' imageRegistry: global.example.com', + 'image:', + ' repository: my-service', + ' registry: sibling.example.com', + '', + ].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('sibling.example.com'); + }); + + it('should fall through to the document-level registry when the sibling registry is templated', () => { + const source = [ + 'global:', + ' imageRegistry: global.example.com', + 'image:', + ' repository: my-service', + ' registry: "{{ .Values.global.registry }}"', + '', + ].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('global.example.com'); + }); + + it('should fall through to the next document-level path when the first holds a non-scalar value', () => { + const source = [ + 'global:', + ' imageRegistry:', + ' host: global.example.com', + 'registry: top.example.com', + 'image:', + ' repository: my-service', + '', + ].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry?.text).toBe('top.example.com'); + }); + + it('should report no registry for a document that declares none', () => { + const source = ['image:', ' repository: docker.io/library/nginx', ' tag: 1.19', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.registry).toBeUndefined(); + }); + + it('should report the registry raw text and source range for a quoted scalar', () => { + const source = ['image:', ' repository: my-service', ' registry: "myreg.example.com"', ''].join('\n'); + + const [reference] = extractImageReferences(source); + const registry = reference?.registry; + + expect(registry).toBeDefined(); + expect(registry!.text).toBe('myreg.example.com'); + expect(source.slice(registry!.range.start, registry!.range.end)).toBe('myreg.example.com'); + }); + + it('should report the repository and tag unchanged for a reference under a declared registry', () => { + const source = ['global:', ' imageRegistry: global.example.com', 'image:', ' repository: my-service', ' tag: 1.10', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('my-service'); + expect(reference?.tag?.text).toBe('1.10'); + expect(reference?.registry?.text).toBe('global.example.com'); + }); + }); + describe('Helm template syntax', () => { it('should produce no reference when the repository contains template syntax', () => { const source = ['image:', ' repository: "{{ .Values.global.registry }}/nginx"', ' tag: 1.19', ''].join('\n'); diff --git a/packages/helm/src/extract-image-references.ts b/packages/helm/src/extract-image-references.ts index c5515c0..8f4c319 100644 --- a/packages/helm/src/extract-image-references.ts +++ b/packages/helm/src/extract-image-references.ts @@ -1,4 +1,4 @@ -import { isPair, isScalar, parseDocument, visit, type Pair } from 'yaml'; +import { isPair, isScalar, parseDocument, visit, type Document, type Pair } from 'yaml'; /** A half-open character range into the original source text. */ interface SourceRange { @@ -20,6 +20,8 @@ interface RawScalar { interface ImageReference { readonly repository: RawScalar; readonly tag: RawScalar | undefined; + /** The registry declared for this reference, if the document declares one. */ + readonly registry: RawScalar | undefined; } /** A sibling key that corroborates `repository` as an image reference. */ @@ -28,6 +30,18 @@ const CORROBORATING_SIBLING_KEYS: ReadonlySet = new Set(['tag', 'pullPol /** The exact parent key that corroborates a `repository` mapping on its own. */ const CORROBORATING_PARENT_KEY = 'image'; +/** + * The key paths under which a values file declares one registry for the whole + * document. Listed in precedence order: the first path holding a usable + * scalar wins, so a new convention is a new row rather than a new branch. + */ +const DOCUMENT_REGISTRY_KEY_PATHS: readonly (readonly string[])[] = [ + ['global', 'imageRegistry'], + ['global', 'registry'], + ['imageRegistry'], + ['registry'], +]; + /** * Finds image references in Helm values file source text. * @@ -38,15 +52,25 @@ const CORROBORATING_PARENT_KEY = 'image'; * ending in `Image`. A missing `tag` doesn't disqualify a candidate; it * makes the reference tagless. * - * `repository` and `tag` are read from raw source text, not the parsed - * value: YAML coerces `1.10` to the float `1.1`, which would put a wrong - * diagnostic on a correct file. + * Values files commonly split the registry off from the repository, so + * `repository: my-service` under a declared `myreg.example.com` names + * `myreg.example.com/my-service`. A sibling `registry` key wins; otherwise + * the document-level registry found through `DOCUMENT_REGISTRY_KEY_PATHS` + * applies to every reference in the file. The declared string is emitted as + * written — deciding what it points at belongs to the OCI registry package. * - * A value containing Helm template syntax is skipped: a templated - * `repository` drops the candidate, a templated `tag` is treated as absent. + * Every field is read from raw source text, not the parsed value: YAML + * coerces `1.10` to the float `1.1`, which would put a wrong diagnostic on a + * correct file. + * + * A value containing Helm template syntax is unusable, and so is a non-scalar + * or empty one. An unusable `repository` drops the candidate; an unusable + * `tag` makes the reference tagless; an unusable `registry` falls through to + * the next candidate in the order above. */ function extractImageReferences(source: string): ImageReference[] { const document = parseDocument(source); + const documentRegistry = readDocumentRegistry(source, document); const references: ImageReference[] = []; visit(document, { @@ -70,18 +94,16 @@ function extractImageReferences(source: string): ImageReference[] { return; } - const repository = readRawScalar(source, repositoryPair.value); + const repository = readUsableScalar(source, repositoryPair.value); - if (repository === undefined || containsHelmTemplateSyntax(repository.text)) { + if (repository === undefined) { return; } - const tagPair = node.items.find((pair) => keyNameOf(pair) === 'tag'); - const tag = tagPair === undefined ? undefined : readRawScalar(source, tagPair.value); - references.push({ repository, - tag: tag === undefined || containsHelmTemplateSyntax(tag.text) ? undefined : tag, + tag: readSiblingScalar(source, node.items, 'tag'), + registry: readSiblingScalar(source, node.items, 'registry') ?? documentRegistry, }); }, }); @@ -89,6 +111,42 @@ function extractImageReferences(source: string): ImageReference[] { return references; } +/** + * Reads the registry the document declares for all of its references, trying + * each known key path in precedence order. + */ +function readDocumentRegistry(source: string, document: Document): RawScalar | undefined { + for (const path of DOCUMENT_REGISTRY_KEY_PATHS) { + // `getIn` hands back parsed values by default; the scalar node is what + // carries the source range. + const registry = readUsableScalar(source, document.getIn(path, true)); + + if (registry !== undefined) { + return registry; + } + } + + return undefined; +} + +/** Reads a named key's value from the mapping a candidate was found in. */ +function readSiblingScalar(source: string, items: readonly Pair[], name: string): RawScalar | undefined { + const pair = items.find((candidate) => keyNameOf(candidate) === name); + + return pair === undefined ? undefined : readUsableScalar(source, pair.value); +} + +/** + * Reads a scalar a consumer can act on. Helm resolves template syntax at + * render time, and this package never renders, so a templated value is no + * more usable here than a missing one and reads as `undefined` too. + */ +function readUsableScalar(source: string, node: unknown): RawScalar | undefined { + const scalar = readRawScalar(source, node); + + return scalar === undefined || containsHelmTemplateSyntax(scalar.text) ? undefined : scalar; +} + /** Whether a mapping's parent key is `image` or ends in `Image`. */ function hasCorroboratingParentKey(path: readonly unknown[]): boolean { const parent = path[path.length - 1]; From a158c7b967fa29b83e424d160b526ba5a9042896 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:05:32 +0300 Subject: [PATCH 2/3] feat(oci-registry): resolve a declared registry, then guess Docker Hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reference that is not fully qualified now gets checked instead of refused. Resolution runs in three steps: a host the repository names itself, then the registry the document declared, then Docker Hub. So `repository: my-service` under a `global.imageRegistry` is checked where the chart says it lives, and `nginx` verifies against Hub with no setup at all — with Docker's own `library/` normalization applied, since the distribution API knows only `library/nginx` and answers the short form with a 404 that reads exactly like a missing image. The third step is a guess, and a guess is not allowed to accuse. A not-found from the Hub fallback is downgraded to unverifiable, because a bare internal service name is the ordinary reference in this organisation's charts and Hub has never heard of any of them, so reporting that as a missing image would be the false negative this whole feature exists to avoid. A positive answer from the guess still stands, so public images keep their checkmark. The downgrade sits on the single verdict the entry point returns rather than on each 404 branch, so no later edit to those branches can make "we guessed the registry" and "we are confident the image is missing" hold at once. `no-registry` is gone: every repository resolves to some registry now, so nothing reaches it. What replaces it is two reasons rather than one, because the old one conflated them. `guessed-registry` is the downgrade above. `malformed-reference` widens from the tag to the whole reference — a repository name outside the OCI grammar, or a declared registry that is not a host, is rejected before a request is built. A declared registry that is not a host is a malformed document, not an invitation to guess Hub instead: falling through would answer a question about a registry nobody asked about. `resolve-explicit-host.ts` becomes `resolve-reference.ts`, one module owning OCI naming. `resolveExplicitHost` keeps its behaviour and its export, because the extension asks it a different question — what host the file itself spells out — to decide whether a checkmark should name the registry that answered. That path is reachable for the first time here, so a bare `nginx` verified on Hub now says `docker.io` on the line rather than claiming a registry the file never mentioned. Closes #23 Co-Authored-By: Claude Opus 5 (1M context) --- apps/vscode/src/diagnostics.test.ts | 2 +- apps/vscode/src/hover.test.ts | 4 +- apps/vscode/src/hover.ts | 4 +- apps/vscode/src/marks.test.ts | 16 ++ apps/vscode/src/reference-check.test.ts | 39 ++++ apps/vscode/src/reference-check.ts | 8 +- packages/oci-registry/src/authorize.ts | 2 +- .../src/check-image-existence.test.ts | 212 +++++++++++++++++- .../oci-registry/src/check-image-existence.ts | 69 ++++-- packages/oci-registry/src/docker-hub.ts | 8 +- packages/oci-registry/src/index.ts | 4 +- .../oci-registry/src/resolve-explicit-host.ts | 50 ----- .../oci-registry/src/resolve-reference.ts | 112 +++++++++ packages/oci-registry/src/verdict.ts | 34 +-- 14 files changed, 466 insertions(+), 98 deletions(-) delete mode 100644 packages/oci-registry/src/resolve-explicit-host.ts create mode 100644 packages/oci-registry/src/resolve-reference.ts diff --git a/apps/vscode/src/diagnostics.test.ts b/apps/vscode/src/diagnostics.test.ts index d199762..7dccb1d 100644 --- a/apps/vscode/src/diagnostics.test.ts +++ b/apps/vscode/src/diagnostics.test.ts @@ -13,7 +13,7 @@ const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, // of going untested. const UNCHECKED_VERDICTS: Record = { 'no-tag': { kind: 'unverifiable', reason: 'no-tag' }, - 'no-registry': { kind: 'unverifiable', reason: 'no-registry' }, + 'guessed-registry': { kind: 'unverifiable', reason: 'guessed-registry' }, 'needs-login': { kind: 'unverifiable', reason: 'needs-login', registry: REPOSITORY }, 'authentication-failure': { kind: 'unverifiable', reason: 'authentication-failure' }, 'network-error': { kind: 'unverifiable', reason: 'network-error' }, diff --git a/apps/vscode/src/hover.test.ts b/apps/vscode/src/hover.test.ts index 02ecd94..adbd2f0 100644 --- a/apps/vscode/src/hover.test.ts +++ b/apps/vscode/src/hover.test.ts @@ -15,12 +15,12 @@ const VERIFIED_VERDICT: ReferenceVerdict = { kind: 'exists', registry: 'mirror.e // the build here instead of hovering with someone else's explanation. const UNCHECKED_REASON_SENTENCES: Record = { 'no-tag': 'the reference names no tag to check.', - 'no-registry': 'the repository names no registry host.', + 'guessed-registry': 'nothing here names a registry, and Docker Hub — the only one left to try — does not have it.', 'needs-login': 'no local Docker credential for that registry. Run `docker login` against it.', 'authentication-failure': 'the registry refused the local Docker credential for it.', 'network-error': 'the registry could not be reached.', 'unexpected-response': 'the registry answered in a form this extension does not understand.', - 'malformed-reference': 'the tag is not a valid OCI tag.', + 'malformed-reference': 'the reference is not a valid image reference.', }; /** The unverifiable verdict a reason produces. Only `needs-login` carries a registry, so the shape cannot be built generically. */ diff --git a/apps/vscode/src/hover.ts b/apps/vscode/src/hover.ts index 642d02d..5094767 100644 --- a/apps/vscode/src/hover.ts +++ b/apps/vscode/src/hover.ts @@ -6,12 +6,12 @@ import { containsOffset } from './source-range'; // instead of hovering with no explanation. const UNCHECKED_REASON_TEXT: Record = { 'no-tag': 'the reference names no tag to check.', - 'no-registry': 'the repository names no registry host.', + 'guessed-registry': 'nothing here names a registry, and Docker Hub — the only one left to try — does not have it.', 'needs-login': 'no local Docker credential for that registry. Run `docker login` against it.', 'authentication-failure': 'the registry refused the local Docker credential for it.', 'network-error': 'the registry could not be reached.', 'unexpected-response': 'the registry answered in a form this extension does not understand.', - 'malformed-reference': 'the tag is not a valid OCI tag.', + 'malformed-reference': 'the reference is not a valid image reference.', }; /** diff --git a/apps/vscode/src/marks.test.ts b/apps/vscode/src/marks.test.ts index a8c9a8c..0ee98d8 100644 --- a/apps/vscode/src/marks.test.ts +++ b/apps/vscode/src/marks.test.ts @@ -82,6 +82,22 @@ describe('marks', () => { expect(otherHost?.renderOptions?.after?.contentText).toBe(' ✓ mirror.example.com'); }); + it('should name the answering registry for a repository that names no host at all', () => { + const bareYaml = ['image:', ' repository: nginx', ' tag: "1.0"', ''].join('\n'); + const document = createFakeDocument('/repo/chart/values.yaml', bareYaml); + const reference = { + repository: { text: 'nginx', range: { start: bareYaml.indexOf('nginx'), end: bareYaml.indexOf('nginx') + 'nginx'.length } }, + tag: undefined, + registry: undefined, + }; + + const [mark] = marksFor(document, [{ reference, verdict: { kind: 'exists', registry: 'docker.io' } }]); + + // A file that spells out no registry cannot have the answer restated to + // it, so naming Docker Hub is the only way the mark says where it looked. + expect(mark?.renderOptions?.after?.contentText).toBe(' ✓ docker.io'); + }); + it('should place the mark on the repository value', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const [mark] = marksFor(document, [createCheck(VERIFIED_VERDICT)]); diff --git a/apps/vscode/src/reference-check.test.ts b/apps/vscode/src/reference-check.test.ts index 03887a6..8cd20ed 100644 --- a/apps/vscode/src/reference-check.test.ts +++ b/apps/vscode/src/reference-check.test.ts @@ -10,6 +10,13 @@ const REPOSITORY = 'docker.io/library/nginx'; const TAG = '1.19'; const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: ${TAG}`, ''].join('\n'); const TAGLESS_VALUES_YAML = ['image:', ' repository: registry.example.com/svc', ' pullPolicy: IfNotPresent', ''].join('\n'); +// A public registry, because a private one is settled as `needs-login` +// before any request and would say nothing about where the check was sent. +const DECLARED_REGISTRY_VALUES_YAML = ['global:', ' imageRegistry: ghcr.io', 'image:', ' repository: my-org/my-service', ' tag: 1.0.0', ''].join( + '\n' +); +const BARE_VALUES_YAML = ['image:', ' repository: my-service', ' tag: 1.0.0', ''].join('\n'); +const PUBLIC_BARE_VALUES_YAML = ['image:', ' repository: nginx', ' tag: 1.19', ''].join('\n'); /** Checks a document this feature is expected to have something to say about. */ async function checkValuesFile(document: vscode.TextDocument, fetch: FetchLike): Promise { @@ -75,6 +82,38 @@ describe('reference-check', () => { expect(fetch).toHaveBeenCalledTimes(1); }); + it('should check a bare repository against the registry its document declares', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', DECLARED_REGISTRY_VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch); + + expect(fetch).toHaveBeenCalledWith('https://ghcr.io/v2/my-org/my-service/manifests/1.0.0', expect.anything()); + expect(checks[0]?.verdict).toEqual({ kind: 'exists', registry: 'ghcr.io' }); + }); + + it('should leave a bare repository Docker Hub has never heard of unverifiable rather than missing', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'NAME_UNKNOWN' }] })); + const document = createFakeDocument('/repo/chart/values.yaml', BARE_VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch); + + // An internal service name is the ordinary case in this organisation's + // charts, and Docker Hub was only ever a guess about where to look. + // `diagnostics.test.ts` is where that reason is pinned to no diagnostic. + expect(checks[0]?.verdict).toEqual({ kind: 'unverifiable', reason: 'guessed-registry' }); + }); + + it('should verify a bare public name against Docker Hub under the library namespace', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', PUBLIC_BARE_VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch); + + expect(fetch).toHaveBeenCalledWith('https://registry-1.docker.io/v2/library/nginx/manifests/1.19', expect.anything()); + expect(checks[0]?.verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + }); + it('should tag the checks with the version of the document they describe', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); diff --git a/apps/vscode/src/reference-check.ts b/apps/vscode/src/reference-check.ts index ddab886..bae7cc1 100644 --- a/apps/vscode/src/reference-check.ts +++ b/apps/vscode/src/reference-check.ts @@ -78,7 +78,13 @@ async function checkImageReferencesInDocument(document: vscode.TextDocument, dep verdict: reference.tag === undefined ? ({ kind: 'unverifiable', reason: 'no-tag' } as const) - : await checkImageExistence({ repository: reference.repository.text, tag: reference.tag.text, fetch, credentials }), + : await checkImageExistence({ + repository: reference.repository.text, + tag: reference.tag.text, + documentRegistry: reference.registry?.text, + fetch, + credentials, + }), })) ); diff --git a/packages/oci-registry/src/authorize.ts b/packages/oci-registry/src/authorize.ts index 62bbc18..70d0872 100644 --- a/packages/oci-registry/src/authorize.ts +++ b/packages/oci-registry/src/authorize.ts @@ -22,7 +22,7 @@ const FORM_CONTENT_TYPE = 'application/x-www-form-urlencoded'; // Hosts a plaintext token endpoint is tolerated on. A local registry has no // certificate and nothing it is told leaves the machine, and `localhost` is -// already a first-class registry host in `resolve-explicit-host`. +// already a first-class registry host in `resolve-reference`. const LOOPBACK_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]']); type BasicCredential = Extract; diff --git a/packages/oci-registry/src/check-image-existence.test.ts b/packages/oci-registry/src/check-image-existence.test.ts index 0ddf8b0..763ac83 100644 --- a/packages/oci-registry/src/check-image-existence.test.ts +++ b/packages/oci-registry/src/check-image-existence.test.ts @@ -93,6 +93,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -111,6 +112,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -132,6 +134,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/does-not-exist', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -145,6 +148,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: 'does-not-exist', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -162,6 +166,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -175,6 +180,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -189,6 +195,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -202,6 +209,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -209,41 +217,214 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'unverifiable', reason: 'unexpected-response' }); }); - it('should report unverifiable without issuing a request when the repository names no explicit host', async () => { + it('should send a repository that names no registry to Docker Hub, under the library namespace a single-segment name lives in', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'nginx', + tag: 'latest', + documentRegistry: undefined, + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://registry-1.docker.io/v2/library/nginx/manifests/latest'); + expect(verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + }); + + it('should prefix the library namespace onto a single-segment name a repository qualifies with docker.io itself', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'docker.io/nginx', + tag: '1.19', + documentRegistry: undefined, + fetch, + credentials: fakeDockerCredentials(), + }); + + // Docker's normalization is a property of Hub, not of how the host was + // arrived at: `docker.io/nginx` addresses the same repository the bare + // `nginx` does. + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://registry-1.docker.io/v2/library/nginx/manifests/1.19'); + expect(verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + }); + + it('should request a repository that names no registry against the one the document declares', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'example/app', + tag: '1.0.0', + documentRegistry: 'ghcr.io', + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://ghcr.io/v2/example/app/manifests/1.0.0'); + expect(verdict).toEqual({ kind: 'exists', registry: 'ghcr.io' }); + }); + + it('should prefer a host the repository names over the registry the document declares', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'quay.io/example/app', + tag: '1.0.0', + documentRegistry: 'ghcr.io', + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://quay.io/v2/example/app/manifests/1.0.0'); + expect(verdict).toEqual({ kind: 'exists', registry: 'quay.io' }); + }); + + it('should treat localhost:5000 as a host the repository names rather than a name on the document registry', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'localhost:5000/svc', + tag: '1.0.0', + documentRegistry: 'ghcr.io', + fetch, + credentials: fakeDockerCredentials({ config: { auths: { 'localhost:5000': { auth: encodeAuth('dev', 's3cret') } } } }), + }); + + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://localhost:5000/v2/svc/manifests/1.0.0'); + expect(verdict).toEqual({ kind: 'exists', registry: 'localhost:5000' }); + }); + + it('should treat a first segment with no dot, colon, or localhost as a namespace rather than a host', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'bitnami/nginx', + tag: '1.19', + documentRegistry: undefined, + fetch, + credentials: fakeDockerCredentials(), + }); + + // `bitnami` is a Hub namespace. Reading it as a host would send the + // request to a machine that does not exist, and the library prefix would + // wrongly apply to a name that already has two segments. + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://registry-1.docker.io/v2/bitnami/nginx/manifests/1.19'); + expect(verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + }); + + it('should report guessed-registry, never tag-not-found, when the guessed Docker Hub answers MANIFEST_UNKNOWN', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 404, body: distributionError('MANIFEST_UNKNOWN') })); + + const verdict = await checkImageExistence({ + repository: 'discrete-agent', + tag: 'v3.2.1', + documentRegistry: undefined, + fetch, + credentials: fakeDockerCredentials(), + }); + + // Nothing in the file named Hub. An internal service absent from it is + // the common case in this organisation's charts, so the 404 is evidence + // about the guess, not about the image. + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'guessed-registry' }); + }); + + it('should report guessed-registry, never repository-not-found, when the guessed Docker Hub answers NAME_UNKNOWN', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 404, body: distributionError('NAME_UNKNOWN') })); + + const verdict = await checkImageExistence({ + repository: 'discrete-agent', + tag: 'v3.2.1', + documentRegistry: undefined, + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'guessed-registry' }); + }); + + it('should report repository-not-found when a document-declared registry answers NAME_UNKNOWN, which is not a guess', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 404, body: distributionError('NAME_UNKNOWN') })); + + const verdict = await checkImageExistence({ + repository: 'example/does-not-exist', + tag: '1.0.0', + documentRegistry: 'ghcr.io', + fetch, + credentials: fakeDockerCredentials(), + }); + + // The document said where to look, so a not-found from there is the + // answer to the question the file asked. Downgrading it too would leave + // the checker unable to report a missing image at all. + expect(verdict).toEqual({ kind: 'repository-not-found', repository: 'example/does-not-exist' }); + }); + + it('should report malformed-reference without issuing a request when the repository fits no valid name grammar', async () => { + const fetch = vi.fn(); + + const verdict = await checkImageExistence({ + repository: 'Not A Name', + tag: '1.0.0', + documentRegistry: 'ghcr.io', + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(fetch).not.toHaveBeenCalled(); + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); + }); + + it('should report malformed-reference without issuing a request when the document registry is not a valid host', async () => { const fetch = vi.fn(); - const verdict = await checkImageExistence({ repository: 'nginx', tag: 'latest', fetch, credentials: fakeDockerCredentials() }); + const verdict = await checkImageExistence({ + repository: 'example/app', + tag: '1.0.0', + documentRegistry: 'https://registry.example.com', + fetch, + credentials: fakeDockerCredentials(), + }); + // Falling back to Hub here would answer a question about a registry the + // document did name, and `https://…` reaches `fetch` as userinfo if it + // is pasted into the URL template unchecked. expect(fetch).not.toHaveBeenCalled(); - expect(verdict).toEqual({ kind: 'unverifiable', reason: 'no-registry' }); + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); }); - it('should report unverifiable without issuing a request when the host segment smuggles userinfo', async () => { + it('should report malformed-reference without issuing a request when the host segment smuggles userinfo', async () => { const fetch = vi.fn(); const verdict = await checkImageExistence({ repository: 'docker.io@evil.example/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); expect(fetch).not.toHaveBeenCalled(); - expect(verdict).toEqual({ kind: 'unverifiable', reason: 'no-registry' }); + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); }); - it('should report unverifiable without issuing a request when a name component is a dot-segment', async () => { + it('should report malformed-reference without issuing a request when a name component is a dot-segment', async () => { const fetch = vi.fn(); const verdict = await checkImageExistence({ repository: 'docker.io/../secrets', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); + // The rejected explicit host must not fall through to the Hub guess: + // the whole string is then offered as a name, and `..` has to fail that + // grammar too or the traversal arrives at Hub instead. expect(fetch).not.toHaveBeenCalled(); - expect(verdict).toEqual({ kind: 'unverifiable', reason: 'no-registry' }); + expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); }); it('should report unverifiable without issuing a request when the tag does not fit the OCI tag grammar', async () => { @@ -252,6 +433,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '../../other', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -276,6 +458,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -315,6 +498,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', auths: { 'https://private.example.com': {} } }, @@ -350,6 +534,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', credHelpers: { 'private.example.com': 'acr-env' } }, @@ -384,6 +569,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -427,6 +613,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'myorg.azurecr.io/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -475,6 +662,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -503,6 +691,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 'expired') } } } }), }); @@ -516,6 +705,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -535,6 +725,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -557,6 +748,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'localhost:5000/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'localhost:5000': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -584,6 +776,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } }, @@ -612,6 +805,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'https://index.docker.io/v1/': { auth: encodeAuth('hub-user', 'hub-secret') } } }, @@ -650,6 +844,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'myorg.azurecr.io/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -677,6 +872,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -714,6 +910,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credHelpers: { 'private.example.com': 'acr-env' } }, runCredentialHelper }), }); @@ -736,6 +933,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', + documentRegistry: undefined, fetch, credentials: fakeDockerCredentials({ configText: `{ "auths": { "private.example.com": { "auth": "${encodeAuth('dev', 's3cret')}" }, } }`, diff --git a/packages/oci-registry/src/check-image-existence.ts b/packages/oci-registry/src/check-image-existence.ts index dfb9946..af00fa4 100644 --- a/packages/oci-registry/src/check-image-existence.ts +++ b/packages/oci-registry/src/check-image-existence.ts @@ -3,7 +3,8 @@ import type { CredentialEnvironment, RegistryCredential } from './credentials'; import { registryEndpoint } from './docker-hub'; import { isPublicRegistry, resolveRegistryCredential } from './credentials'; import type { FetchLike, FetchResponseLike } from './fetch-like'; -import { resolveExplicitHost } from './resolve-explicit-host'; +import { resolveReference } from './resolve-reference'; +import type { ResolvedReference } from './resolve-reference'; import type { ImageVerdict } from './verdict'; /** @@ -31,6 +32,13 @@ const TAG_PATTERN = /^[a-zA-Z0-9_][a-zA-Z0-9_.-]{0,127}$/; interface CheckImageExistenceParams { readonly repository: string; readonly tag: string; + /** + * A registry declared elsewhere in the same document, or `undefined` when + * the document declares none. Required rather than optional so that a + * caller with nothing to say has to say so, instead of a forgotten field + * silently downgrading a resolvable reference into a Docker Hub guess. + */ + readonly documentRegistry: string | undefined; readonly fetch: FetchLike; readonly credentials: CredentialEnvironment; } @@ -38,32 +46,59 @@ interface CheckImageExistenceParams { /** * Checks whether an image reference exists on a container registry. * - * This is the package's single entry point: everything else — host - * detection, credential resolution, the request shape, the distinction + * This is the package's single entry point: everything else — registry + * resolution, credential resolution, the request shape, the distinction * between a missing repository and a missing tag — is reached only through * here, on purpose, so a test asserts the requests issued and the verdict * returned rather than an internal function. * - * Only a fully-qualified repository (one naming an explicit registry host) - * is supported so far — no document/registry fallback, no Docker Hub - * fallback. Credentials come from the local Docker config and nowhere else: - * this package never prompts, and never invents one. Anything it cannot - * resolve, reach, or interpret comes back as `'unverifiable'`, never as a - * false negative. + * The registry is resolved in three steps: a host the repository names + * itself, else `documentRegistry`, else Docker Hub. That last step is a + * guess, so a not-found from it is downgraded to `'guessed-registry'`, for + * the reason recorded on that member of {@link UnverifiableReason}. A + * positive answer from the guess still stands, so public images keep + * verifying. + * + * Credentials come from the local Docker config and nowhere else: this + * package never prompts, and never invents one. Anything it cannot resolve, + * reach, or interpret comes back as `'unverifiable'`, never as a false + * negative. */ async function checkImageExistence(params: CheckImageExistenceParams): Promise { - const { repository, tag, fetch, credentials } = params; - const location = resolveExplicitHost(repository); - - if (location === undefined) { - return { kind: 'unverifiable', reason: 'no-registry' }; - } + const { repository, tag, documentRegistry, fetch, credentials } = params; + const reference = resolveReference(repository, documentRegistry); - if (!TAG_PATTERN.test(tag)) { + if (reference === undefined || !TAG_PATTERN.test(tag)) { return { kind: 'unverifiable', reason: 'malformed-reference' }; } - const { host, name } = location; + const verdict = await checkResolvedReference({ repository, reference, tag, fetch, credentials }); + const registryWasGuessed = reference.source === 'docker-hub-fallback'; + const answeredNotFound = verdict.kind === 'repository-not-found' || verdict.kind === 'tag-not-found'; + + return registryWasGuessed && answeredNotFound ? { kind: 'unverifiable', reason: 'guessed-registry' } : verdict; +} + +interface CheckResolvedReferenceParams { + /** The repository as the file wrote it, which is what a not-found verdict names. */ + readonly repository: string; + readonly reference: ResolvedReference; + readonly tag: string; + readonly fetch: FetchLike; + readonly credentials: CredentialEnvironment; +} + +/** + * Asks one registry about one reference. + * + * Split out so the guess-downgrade applies to a single returned verdict + * rather than to each not-found branch below. Spread across the branches, + * the rule would be one edit away from a 404 path that reports a missing + * image on the strength of a registry nobody named. + */ +async function checkResolvedReference(params: CheckResolvedReferenceParams): Promise { + const { repository, reference, tag, fetch, credentials } = params; + const { host, name } = reference; const credential = await resolveRegistryCredential(host, credentials); // Resolved before the first request, not after a 401, because an diff --git a/packages/oci-registry/src/docker-hub.ts b/packages/oci-registry/src/docker-hub.ts index 8beb8f7..5ee6240 100644 --- a/packages/oci-registry/src/docker-hub.ts +++ b/packages/oci-registry/src/docker-hub.ts @@ -22,6 +22,12 @@ const DOCKER_HUB_ENDPOINT = 'registry-1.docker.io'; // bare host would be a cache miss in every helper a real developer has. const DOCKER_HUB_SERVER_URL = 'https://index.docker.io/v1/'; +// The namespace Hub files an official image under. `nginx` is only ever a +// spelling of `library/nginx`; the distribution API knows the long form +// alone and answers the short one with a 404 that reads exactly like a +// missing image. +const DOCKER_HUB_LIBRARY_NAMESPACE = 'library'; + const DOCKER_HUB_ALIASES = new Set([DOCKER_HUB_HOST, 'index.docker.io', DOCKER_HUB_ENDPOINT]); function isDockerHub(host: string): boolean { @@ -40,4 +46,4 @@ function registryEndpoint(host: string): string { return isDockerHub(host) ? DOCKER_HUB_ENDPOINT : host; } -export { DOCKER_HUB_HOST, DOCKER_HUB_SERVER_URL, isDockerHub, registryEndpoint }; +export { DOCKER_HUB_HOST, DOCKER_HUB_LIBRARY_NAMESPACE, DOCKER_HUB_SERVER_URL, isDockerHub, registryEndpoint }; diff --git a/packages/oci-registry/src/index.ts b/packages/oci-registry/src/index.ts index 2c30b0b..b990cd5 100644 --- a/packages/oci-registry/src/index.ts +++ b/packages/oci-registry/src/index.ts @@ -3,6 +3,6 @@ export type { CheckImageExistenceParams } from './check-image-existence'; export type { CredentialEnvironment } from './credentials'; export type { FetchLike, FetchRequestInit, FetchResponseLike } from './fetch-like'; export { localDockerCredentials } from './local-docker-credentials'; -export { resolveExplicitHost } from './resolve-explicit-host'; -export type { RepositoryLocation } from './resolve-explicit-host'; +export { resolveExplicitHost } from './resolve-reference'; +export type { RepositoryLocation } from './resolve-reference'; export type { ImageVerdict, UnverifiableReason } from './verdict'; diff --git a/packages/oci-registry/src/resolve-explicit-host.ts b/packages/oci-registry/src/resolve-explicit-host.ts deleted file mode 100644 index 1128028..0000000 --- a/packages/oci-registry/src/resolve-explicit-host.ts +++ /dev/null @@ -1,50 +0,0 @@ -// A registry host: DNS-label characters and dots, with an optional numeric -// port. Anything outside this set — most pointedly `@`, which a raw -// `https://${host}/...` template would let a crafted repository string use -// to smuggle a different host into the request via URL userinfo — is -// rejected rather than sent to `fetch`. -const HOST_PATTERN = /^[a-zA-Z0-9.-]+(?::[0-9]+)?$/; - -// The OCI distribution spec's `name` grammar: one or more lowercase -// path components, each starting and ending alphanumeric, joined by `/`. -// A component can never be `.` or `..`, which is what keeps a crafted name -// from collapsing the manifest URL's path onto a neighbouring endpoint. -const NAME_PATTERN = /^[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*(?:\/[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*)*$/; - -/** Where a repository resolved to: a registry host, and the image's name on it. */ -export interface RepositoryLocation { - readonly host: string; - readonly name: string; -} - -/** - * Detects an explicit registry host named in a repository string, using the - * standard OCI/Docker rule: the first `/`-separated segment counts as a - * host when it contains a dot or a colon, or is exactly `localhost`. - * - * Reference normalization is OCI naming semantics, not anything Helm- or - * editor-specific, which is why it lives here rather than in the package - * that extracted the raw string. This is also the only registry-resolution - * step this package implements today: a registry declared elsewhere in the - * same YAML document, a workspace override set, and the Docker Hub fallback - * are later tickets. Returning `undefined` when no explicit host is found - * — rather than guessing one — is what lets the caller treat "not fully - * qualified" as unverifiable instead of inventing a wrong answer. - */ -export function resolveExplicitHost(repository: string): RepositoryLocation | undefined { - const segments = repository.split('/'); - const [firstSegment] = segments; - const name = segments.slice(1).join('/'); - - if (firstSegment === undefined || name === '') { - return undefined; - } - - const looksLikeHost = firstSegment === 'localhost' || firstSegment.includes('.') || firstSegment.includes(':'); - - if (!looksLikeHost || !HOST_PATTERN.test(firstSegment) || !NAME_PATTERN.test(name)) { - return undefined; - } - - return { host: firstSegment, name }; -} diff --git a/packages/oci-registry/src/resolve-reference.ts b/packages/oci-registry/src/resolve-reference.ts new file mode 100644 index 0000000..e029b24 --- /dev/null +++ b/packages/oci-registry/src/resolve-reference.ts @@ -0,0 +1,112 @@ +import { DOCKER_HUB_HOST, DOCKER_HUB_LIBRARY_NAMESPACE, isDockerHub } from './docker-hub'; + +// A registry host: DNS-label characters and dots, with an optional numeric +// port. Anything outside this set — most pointedly `@`, which a raw +// `https://${host}/...` template would let a crafted repository string use +// to smuggle a different host into the request via URL userinfo — is +// rejected rather than sent to `fetch`. +const HOST_PATTERN = /^[a-zA-Z0-9.-]+(?::[0-9]+)?$/; + +// The OCI distribution spec's `name` grammar: one or more lowercase +// path components, each starting and ending alphanumeric, joined by `/`. +// A component can never be `.` or `..`, which is what keeps a crafted name +// from collapsing the manifest URL's path onto a neighbouring endpoint. +const NAME_PATTERN = /^[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*(?:\/[a-z0-9]+(?:(?:[._]|__|-+)[a-z0-9]+)*)*$/; + +/** Where a repository resolved to: a registry host, and the image's name on it. */ +interface RepositoryLocation { + readonly host: string; + readonly name: string; +} + +/** How the registry a reference was checked against was arrived at. */ +type RegistrySource = 'explicit' | 'document' | 'docker-hub-fallback'; + +/** A repository string resolved to a registry host and a name on it, kept with how that host was arrived at. */ +interface ResolvedReference { + readonly host: string; + readonly name: string; + readonly source: RegistrySource; +} + +/** + * Detects an explicit registry host named in a repository string, using the + * standard OCI/Docker rule: the first `/`-separated segment counts as a + * host when it contains a dot or a colon, or is exactly `localhost`. + * + * Reference normalization is OCI naming semantics, not anything Helm- or + * editor-specific, which is why it lives here rather than in the package + * that extracted the raw string. This answers only what the string itself + * names, which is the first step of {@link resolveReference} and separately + * the whole question an editor asks when it wants to know whether a file + * spelled its registry out. `undefined` therefore means "no explicit host", + * not "no registry": deciding what to do about that is the caller's. + */ +function resolveExplicitHost(repository: string): RepositoryLocation | undefined { + const segments = repository.split('/'); + const [firstSegment] = segments; + const name = segments.slice(1).join('/'); + + if (firstSegment === undefined || name === '') { + return undefined; + } + + const looksLikeHost = firstSegment === 'localhost' || firstSegment.includes('.') || firstSegment.includes(':'); + + if (!looksLikeHost || !HOST_PATTERN.test(firstSegment) || !NAME_PATTERN.test(name)) { + return undefined; + } + + return { host: firstSegment, name }; +} + +/** + * Resolves a repository string to the registry its image would be pulled + * from: a host the string names itself, else a registry declared elsewhere + * in the same document, else Docker Hub. + * + * `undefined` means the reference is malformed — a name outside the OCI + * grammar, or a `documentRegistry` that is not a host — and never "no + * registry was found", because the last step always produces one. That last + * step is a guess, which is why the answer carries its + * {@link ResolvedReference.source}: a not-found from a registry nobody named + * says something about the guess rather than about the image, and only the + * source distinguishes the two. + */ +function resolveReference(repository: string, documentRegistry: string | undefined): ResolvedReference | undefined { + const reference = selectRegistry(repository, documentRegistry); + + if (reference === undefined || !isDockerHub(reference.host) || reference.name.includes('/')) { + return reference; + } + + // Docker's own normalization, applied however the host was arrived at: + // `nginx`, `docker.io/nginx` and a document registry of `docker.io` all + // address `library/nginx`, and Hub serves the long form only. + return { ...reference, name: `${DOCKER_HUB_LIBRARY_NAMESPACE}/${reference.name}` }; +} + +/** Picks which of the three registry sources applies, before Hub name normalization. */ +function selectRegistry(repository: string, documentRegistry: string | undefined): ResolvedReference | undefined { + const explicit = resolveExplicitHost(repository); + + if (explicit !== undefined) { + return { ...explicit, source: 'explicit' }; + } + + if (!NAME_PATTERN.test(repository)) { + return undefined; + } + + if (documentRegistry === undefined || documentRegistry === '') { + return { host: DOCKER_HUB_HOST, name: repository, source: 'docker-hub-fallback' }; + } + + // A declared registry that is not a host is a malformed document, not an + // invitation to guess Hub instead: falling through would answer a + // question about a registry the document never asked about. + return HOST_PATTERN.test(documentRegistry) ? { host: documentRegistry, name: repository, source: 'document' } : undefined; +} + +export { resolveExplicitHost, resolveReference }; +export type { RepositoryLocation, ResolvedReference }; diff --git a/packages/oci-registry/src/verdict.ts b/packages/oci-registry/src/verdict.ts index b78039c..e2549cf 100644 --- a/packages/oci-registry/src/verdict.ts +++ b/packages/oci-registry/src/verdict.ts @@ -1,18 +1,19 @@ /** * The reason an image's existence could not be determined. * - * This list is expected to grow — the document/registry fallback chain and - * the Docker Hub guess-downgrade rule from spec #17 each add reasons of - * their own — but every reason, present or future, resolves to the same - * {@link ImageVerdict} `'unverifiable'` kind. That is the whole point of the - * shape: no caller can special-case a *reason* into rendering a diagnostic, - * because only the verdict `kind` controls that. + * Every reason resolves to the same {@link ImageVerdict} `'unverifiable'` + * kind. That is the whole point of the shape: no caller can special-case a + * *reason* into rendering a diagnostic, because only the verdict `kind` + * controls that. */ type UnverifiableReason = - /** The repository names no explicit registry host, and this package does - * not yet resolve one any other way (a document-declared registry, a - * workspace override set, or the Docker Hub fallback). */ - | 'no-registry' + /** The repository named no registry and the document declared none, so + * Docker Hub was guessed — and the guess answered not-found. That is no + * evidence the image is missing, only that this tool never knew where to + * look. A bare internal service name absent from Hub is the ordinary case + * in this organisation's charts, so reporting it as missing would be the + * false negative this reason exists to prevent. */ + | 'guessed-registry' /** The registry is not one of the public ones, and no credential for it * could be resolved from the local Docker config. Distinct from * `'authentication-failure'` on purpose: this is the one reason a caller @@ -29,9 +30,11 @@ type UnverifiableReason = | 'network-error' /** The registry responded, but not in a way this checker understands. */ | 'unexpected-response' - /** The tag doesn't fit the OCI tag grammar. Rejected before any request is - * issued — building a manifest URL from an unvalidated tag is how a - * crafted values file smuggles a path traversal into the request. */ + /** Some part of the reference is outside the grammar it has to satisfy: the + * tag, the repository name, or a registry the document declared. Rejected + * before any request is issued — building a manifest URL out of an + * unvalidated reference is how a crafted values file smuggles a path + * traversal, or a different host, into the request. */ | 'malformed-reference'; /** @@ -54,7 +57,10 @@ type UnverifiableVerdict = * because the two failure modes are not interchangeable: `'unverifiable'` * must never be treated as evidence the image is missing. That invariant — * an unverifiable verdict produces no diagnostic — is the reason this type - * exists in this shape rather than a simpler one. + * exists in this shape rather than a simpler one, and it is what the + * `'guessed-registry'` downgrade relies on: a not-found this package does + * not trust is moved into the kind that says nothing, which is only a safe + * move because that kind is guaranteed to stay silent. */ type ImageVerdict = | { readonly kind: 'exists'; readonly registry: string } From 2c1f46abe501aa326bd5d32bf929ba57047477a1 Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:11:23 +0300 Subject: [PATCH 3/3] fix(vscode): stop the checkmark restating a registry the file declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `registrySuffixOf` names the answering registry only when it differs from the one the file names, and it was still asking only what the repository string spells out — a question this branch had just made the wrong one. A values file declaring `global.imageRegistry: ghcr.io` got ` ✓ ghcr.io` appended to every bare repository under it, which is the wallpaper that rule exists to prevent. The comparison now runs against the registry the file names by either route. A file naming none still gets Docker Hub spelled out, because "we guessed" and "you wrote it" are different things and the mark is where that difference shows. The rest is review fallout from the same pass. `documentRegistry` becomes `declaredRegistry`, and its `RegistrySource` arm `'document'` becomes `'declared'`. ADR 0001 says the registry package knows nothing about Helm, YAML, or editors, and a document is the caller's idea, not this package's. No dependency crossed that line, only vocabulary, which is how it gets crossed for real later. `ImageReference.registry` narrows from a `RawScalar` to a plain string. Only its text was ever read, and its range was a hazard rather than a spare feature: a document-level declaration sits on a line that has nothing to do with the reference, so the first consumer to underline it would have underlined `global:` for a diagnostic about an image forty lines down. `ResolvedReference` extends `RepositoryLocation` instead of respelling its two fields. The empty-string branch in `selectRegistry` keeps its fallback and gains the test and the comment it was missing: an empty declared registry is a caller reading a half-typed file saying "nothing declared", not a registry named badly, so it falls back rather than failing. A `{@link UnverifiableReason}` in `check-image-existence.ts` pointed at a symbol that file does not import, so it resolved to nothing. Both new assertions were mutation-checked against the code they cover. Refs #23 Co-Authored-By: Claude Opus 5 (1M context) --- apps/vscode/src/marks.test.ts | 16 +++ apps/vscode/src/marks.ts | 14 ++- apps/vscode/src/reference-check.ts | 2 +- .../helm/src/extract-image-references.test.ts | 23 ++-- packages/helm/src/extract-image-references.ts | 20 ++-- .../src/check-image-existence.test.ts | 105 ++++++++++-------- .../oci-registry/src/check-image-existence.ts | 24 ++-- .../oci-registry/src/resolve-reference.ts | 33 +++--- packages/oci-registry/src/verdict.ts | 4 +- 9 files changed, 143 insertions(+), 98 deletions(-) diff --git a/apps/vscode/src/marks.test.ts b/apps/vscode/src/marks.test.ts index 0ee98d8..3e3377e 100644 --- a/apps/vscode/src/marks.test.ts +++ b/apps/vscode/src/marks.test.ts @@ -82,6 +82,22 @@ describe('marks', () => { expect(otherHost?.renderOptions?.after?.contentText).toBe(' ✓ mirror.example.com'); }); + it('should stay silent when the answering registry is the one the file declared', () => { + const declaredYaml = ['global:', ' imageRegistry: ghcr.io', 'image:', ' repository: my-org/svc', ' tag: "1.0"', ''].join('\n'); + const document = createFakeDocument('/repo/chart/values.yaml', declaredYaml); + const start = declaredYaml.indexOf('my-org/svc'); + const reference = { + repository: { text: 'my-org/svc', range: { start, end: start + 'my-org/svc'.length } }, + tag: undefined, + registry: 'ghcr.io', + }; + + const [mark] = marksFor(document, [{ reference, verdict: { kind: 'exists', registry: 'ghcr.io' } }]); + + // The file says `ghcr.io` two lines up. Appending it restates the file. + expect(mark?.renderOptions?.after?.contentText).toBe(' ✓'); + }); + it('should name the answering registry for a repository that names no host at all', () => { const bareYaml = ['image:', ' repository: nginx', ' tag: "1.0"', ''].join('\n'); const document = createFakeDocument('/repo/chart/values.yaml', bareYaml); diff --git a/apps/vscode/src/marks.ts b/apps/vscode/src/marks.ts index 547f659..42a2aaf 100644 --- a/apps/vscode/src/marks.ts +++ b/apps/vscode/src/marks.ts @@ -29,11 +29,21 @@ function markKindOf(verdict: ReferenceVerdict): MarkKind { } /** - * The answering registry, named only when it differs from the host the file + * The registry the file itself names for a reference: a host spelled out in + * the repository, else the one the document declared for it. `undefined` when the + * file names none, which is not the same as naming Docker Hub — the check + * only guessed it, and the mark is the one place that difference shows. + */ +function registryNamedByFile(reference: ImageReference): string | undefined { + return resolveExplicitHost(reference.repository.text)?.host ?? reference.registry; +} + +/** + * The answering registry, named only when it differs from the one the file * names, so the mark carries information instead of restating the line. */ function registrySuffixOf(reference: ImageReference, verdict: ReferenceVerdict): string { - if (verdict.kind !== 'exists' || verdict.registry === resolveExplicitHost(reference.repository.text)?.host) { + if (verdict.kind !== 'exists' || verdict.registry === registryNamedByFile(reference)) { return ''; } diff --git a/apps/vscode/src/reference-check.ts b/apps/vscode/src/reference-check.ts index bae7cc1..c657eff 100644 --- a/apps/vscode/src/reference-check.ts +++ b/apps/vscode/src/reference-check.ts @@ -81,7 +81,7 @@ async function checkImageReferencesInDocument(document: vscode.TextDocument, dep : await checkImageExistence({ repository: reference.repository.text, tag: reference.tag.text, - documentRegistry: reference.registry?.text, + declaredRegistry: reference.registry, fetch, credentials, }), diff --git a/packages/helm/src/extract-image-references.test.ts b/packages/helm/src/extract-image-references.test.ts index 42fb404..ba6bad4 100644 --- a/packages/helm/src/extract-image-references.test.ts +++ b/packages/helm/src/extract-image-references.test.ts @@ -178,7 +178,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('sibling.example.com'); + expect(reference?.registry).toBe('sibling.example.com'); }); it('should apply a top-level registry key to a reference with no sibling registry', () => { @@ -186,7 +186,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('top.example.com'); + expect(reference?.registry).toBe('top.example.com'); }); it('should prefer global.imageRegistry over a top-level registry', () => { @@ -196,7 +196,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('global.example.com'); + expect(reference?.registry).toBe('global.example.com'); }); it('should apply global.registry when global.imageRegistry is absent', () => { @@ -204,7 +204,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('global.example.com'); + expect(reference?.registry).toBe('global.example.com'); }); it('should prefer a sibling registry over a document-level one', () => { @@ -219,7 +219,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('sibling.example.com'); + expect(reference?.registry).toBe('sibling.example.com'); }); it('should fall through to the document-level registry when the sibling registry is templated', () => { @@ -234,7 +234,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('global.example.com'); + expect(reference?.registry).toBe('global.example.com'); }); it('should fall through to the next document-level path when the first holds a non-scalar value', () => { @@ -250,7 +250,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.registry?.text).toBe('top.example.com'); + expect(reference?.registry).toBe('top.example.com'); }); it('should report no registry for a document that declares none', () => { @@ -261,15 +261,12 @@ describe('extractImageReferences', () => { expect(reference?.registry).toBeUndefined(); }); - it('should report the registry raw text and source range for a quoted scalar', () => { + it('should report the registry as written, with one layer of quotes stripped', () => { const source = ['image:', ' repository: my-service', ' registry: "myreg.example.com"', ''].join('\n'); const [reference] = extractImageReferences(source); - const registry = reference?.registry; - expect(registry).toBeDefined(); - expect(registry!.text).toBe('myreg.example.com'); - expect(source.slice(registry!.range.start, registry!.range.end)).toBe('myreg.example.com'); + expect(reference?.registry).toBe('myreg.example.com'); }); it('should report the repository and tag unchanged for a reference under a declared registry', () => { @@ -279,7 +276,7 @@ describe('extractImageReferences', () => { expect(reference?.repository.text).toBe('my-service'); expect(reference?.tag?.text).toBe('1.10'); - expect(reference?.registry?.text).toBe('global.example.com'); + expect(reference?.registry).toBe('global.example.com'); }); }); diff --git a/packages/helm/src/extract-image-references.ts b/packages/helm/src/extract-image-references.ts index 8f4c319..f2e9498 100644 --- a/packages/helm/src/extract-image-references.ts +++ b/packages/helm/src/extract-image-references.ts @@ -20,8 +20,14 @@ interface RawScalar { interface ImageReference { readonly repository: RawScalar; readonly tag: RawScalar | undefined; - /** The registry declared for this reference, if the document declares one. */ - readonly registry: RawScalar | undefined; + /** + * The registry declared for this reference, as written, or `undefined` + * when the document declares none. A bare string rather than a + * {@link RawScalar}: a document-level declaration sits on a line that has + * nothing to do with this reference, so a range here would underline + * somewhere misleading. + */ + readonly registry: string | undefined; } /** A sibling key that corroborates `repository` as an image reference. */ @@ -103,7 +109,7 @@ function extractImageReferences(source: string): ImageReference[] { references.push({ repository, tag: readSiblingScalar(source, node.items, 'tag'), - registry: readSiblingScalar(source, node.items, 'registry') ?? documentRegistry, + registry: readSiblingScalar(source, node.items, 'registry')?.text ?? documentRegistry, }); }, }); @@ -115,14 +121,14 @@ function extractImageReferences(source: string): ImageReference[] { * Reads the registry the document declares for all of its references, trying * each known key path in precedence order. */ -function readDocumentRegistry(source: string, document: Document): RawScalar | undefined { +function readDocumentRegistry(source: string, document: Document): string | undefined { for (const path of DOCUMENT_REGISTRY_KEY_PATHS) { - // `getIn` hands back parsed values by default; the scalar node is what - // carries the source range. + // `getIn` hands back parsed values by default, and a parsed value is the + // one thing this package never reads a field from. const registry = readUsableScalar(source, document.getIn(path, true)); if (registry !== undefined) { - return registry; + return registry.text; } } diff --git a/packages/oci-registry/src/check-image-existence.test.ts b/packages/oci-registry/src/check-image-existence.test.ts index 763ac83..f429cf4 100644 --- a/packages/oci-registry/src/check-image-existence.test.ts +++ b/packages/oci-registry/src/check-image-existence.test.ts @@ -93,7 +93,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -112,7 +112,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -134,7 +134,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/does-not-exist', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -148,7 +148,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: 'does-not-exist', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -166,7 +166,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -180,7 +180,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -195,7 +195,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -209,7 +209,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -223,7 +223,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'nginx', tag: 'latest', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -238,7 +238,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -250,13 +250,13 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); }); - it('should request a repository that names no registry against the one the document declares', async () => { + it('should request a repository that names no registry against the one declared for it', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); const verdict = await checkImageExistence({ repository: 'example/app', tag: '1.0.0', - documentRegistry: 'ghcr.io', + declaredRegistry: 'ghcr.io', fetch, credentials: fakeDockerCredentials(), }); @@ -265,13 +265,13 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'exists', registry: 'ghcr.io' }); }); - it('should prefer a host the repository names over the registry the document declares', async () => { + it('should prefer a host the repository names over a declared registry', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); const verdict = await checkImageExistence({ repository: 'quay.io/example/app', tag: '1.0.0', - documentRegistry: 'ghcr.io', + declaredRegistry: 'ghcr.io', fetch, credentials: fakeDockerCredentials(), }); @@ -280,13 +280,13 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'exists', registry: 'quay.io' }); }); - it('should treat localhost:5000 as a host the repository names rather than a name on the document registry', async () => { + it('should treat localhost:5000 as a host the repository names rather than a name on the declared registry', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); const verdict = await checkImageExistence({ repository: 'localhost:5000/svc', tag: '1.0.0', - documentRegistry: 'ghcr.io', + declaredRegistry: 'ghcr.io', fetch, credentials: fakeDockerCredentials({ config: { auths: { 'localhost:5000': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -301,7 +301,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'bitnami/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -319,7 +319,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'discrete-agent', tag: 'v3.2.1', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -336,7 +336,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'discrete-agent', tag: 'v3.2.1', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -344,18 +344,18 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'unverifiable', reason: 'guessed-registry' }); }); - it('should report repository-not-found when a document-declared registry answers NAME_UNKNOWN, which is not a guess', async () => { + it('should report repository-not-found when a declared registry answers NAME_UNKNOWN, which is not a guess', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 404, body: distributionError('NAME_UNKNOWN') })); const verdict = await checkImageExistence({ repository: 'example/does-not-exist', tag: '1.0.0', - documentRegistry: 'ghcr.io', + declaredRegistry: 'ghcr.io', fetch, credentials: fakeDockerCredentials(), }); - // The document said where to look, so a not-found from there is the + // The caller said where to look, so a not-found from there is the // answer to the question the file asked. Downgrading it too would leave // the checker unable to report a missing image at all. expect(verdict).toEqual({ kind: 'repository-not-found', repository: 'example/does-not-exist' }); @@ -367,7 +367,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'Not A Name', tag: '1.0.0', - documentRegistry: 'ghcr.io', + declaredRegistry: 'ghcr.io', fetch, credentials: fakeDockerCredentials(), }); @@ -376,19 +376,34 @@ describe('checkImageExistence', () => { expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); }); - it('should report malformed-reference without issuing a request when the document registry is not a valid host', async () => { + it('should treat an empty declared registry as nothing declared rather than as a registry named badly', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse({ status: 200, body: { schemaVersion: 2 } })); + + const verdict = await checkImageExistence({ + repository: 'example/app', + tag: '1.0.0', + declaredRegistry: '', + fetch, + credentials: fakeDockerCredentials(), + }); + + expect(requestAt(fetch.mock.calls, 0).url).toBe('https://registry-1.docker.io/v2/example/app/manifests/1.0.0'); + expect(verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); + }); + + it('should report malformed-reference without issuing a request when the declared registry is not a valid host', async () => { const fetch = vi.fn(); const verdict = await checkImageExistence({ repository: 'example/app', tag: '1.0.0', - documentRegistry: 'https://registry.example.com', + declaredRegistry: 'https://registry.example.com', fetch, credentials: fakeDockerCredentials(), }); // Falling back to Hub here would answer a question about a registry the - // document did name, and `https://…` reaches `fetch` as userinfo if it + // caller did name, and `https://…` reaches `fetch` as userinfo if it // is pasted into the URL template unchecked. expect(fetch).not.toHaveBeenCalled(); expect(verdict).toEqual({ kind: 'unverifiable', reason: 'malformed-reference' }); @@ -400,7 +415,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io@evil.example/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -415,7 +430,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/../secrets', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -433,7 +448,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '../../other', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -458,7 +473,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -498,7 +513,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', auths: { 'https://private.example.com': {} } }, @@ -534,7 +549,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', credHelpers: { 'private.example.com': 'acr-env' } }, @@ -569,7 +584,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -613,7 +628,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'myorg.azurecr.io/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -662,7 +677,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -691,7 +706,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 'expired') } } } }), }); @@ -705,7 +720,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'ghcr.io/example/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials(), }); @@ -725,7 +740,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -748,7 +763,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'localhost:5000/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'localhost:5000': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -776,7 +791,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credsStore: 'desktop', auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } }, @@ -805,7 +820,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'docker.io/library/nginx', tag: '1.19', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'https://index.docker.io/v1/': { auth: encodeAuth('hub-user', 'hub-secret') } } }, @@ -844,7 +859,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'myorg.azurecr.io/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { @@ -872,7 +887,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { auths: { 'private.example.com': { auth: encodeAuth('dev', 's3cret') } } } }), }); @@ -910,7 +925,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ config: { credHelpers: { 'private.example.com': 'acr-env' } }, runCredentialHelper }), }); @@ -933,7 +948,7 @@ describe('checkImageExistence', () => { const verdict = await checkImageExistence({ repository: 'private.example.com/app', tag: '1.0.0', - documentRegistry: undefined, + declaredRegistry: undefined, fetch, credentials: fakeDockerCredentials({ configText: `{ "auths": { "private.example.com": { "auth": "${encodeAuth('dev', 's3cret')}" }, } }`, diff --git a/packages/oci-registry/src/check-image-existence.ts b/packages/oci-registry/src/check-image-existence.ts index af00fa4..0ab77bf 100644 --- a/packages/oci-registry/src/check-image-existence.ts +++ b/packages/oci-registry/src/check-image-existence.ts @@ -33,12 +33,13 @@ interface CheckImageExistenceParams { readonly repository: string; readonly tag: string; /** - * A registry declared elsewhere in the same document, or `undefined` when - * the document declares none. Required rather than optional so that a - * caller with nothing to say has to say so, instead of a forgotten field - * silently downgrading a resolvable reference into a Docker Hub guess. + * A registry the caller found declared for this reference somewhere the + * repository string itself does not reach, or `undefined` when there is + * none. Required rather than optional so that a caller with nothing to say + * has to say so, instead of a forgotten field silently downgrading a + * resolvable reference into a Docker Hub guess. */ - readonly documentRegistry: string | undefined; + readonly declaredRegistry: string | undefined; readonly fetch: FetchLike; readonly credentials: CredentialEnvironment; } @@ -53,11 +54,10 @@ interface CheckImageExistenceParams { * returned rather than an internal function. * * The registry is resolved in three steps: a host the repository names - * itself, else `documentRegistry`, else Docker Hub. That last step is a - * guess, so a not-found from it is downgraded to `'guessed-registry'`, for - * the reason recorded on that member of {@link UnverifiableReason}. A - * positive answer from the guess still stands, so public images keep - * verifying. + * itself, else `declaredRegistry`, else Docker Hub. That last step is a + * guess, so a not-found from it is downgraded to `'guessed-registry'`, whose + * own doc comment in `verdict.ts` records why. A positive answer from the + * guess still stands, so public images keep verifying. * * Credentials come from the local Docker config and nowhere else: this * package never prompts, and never invents one. Anything it cannot resolve, @@ -65,8 +65,8 @@ interface CheckImageExistenceParams { * negative. */ async function checkImageExistence(params: CheckImageExistenceParams): Promise { - const { repository, tag, documentRegistry, fetch, credentials } = params; - const reference = resolveReference(repository, documentRegistry); + const { repository, tag, declaredRegistry, fetch, credentials } = params; + const reference = resolveReference(repository, declaredRegistry); if (reference === undefined || !TAG_PATTERN.test(tag)) { return { kind: 'unverifiable', reason: 'malformed-reference' }; diff --git a/packages/oci-registry/src/resolve-reference.ts b/packages/oci-registry/src/resolve-reference.ts index e029b24..edb461d 100644 --- a/packages/oci-registry/src/resolve-reference.ts +++ b/packages/oci-registry/src/resolve-reference.ts @@ -20,12 +20,10 @@ interface RepositoryLocation { } /** How the registry a reference was checked against was arrived at. */ -type RegistrySource = 'explicit' | 'document' | 'docker-hub-fallback'; +type RegistrySource = 'explicit' | 'declared' | 'docker-hub-fallback'; /** A repository string resolved to a registry host and a name on it, kept with how that host was arrived at. */ -interface ResolvedReference { - readonly host: string; - readonly name: string; +interface ResolvedReference extends RepositoryLocation { readonly source: RegistrySource; } @@ -62,32 +60,32 @@ function resolveExplicitHost(repository: string): RepositoryLocation | undefined /** * Resolves a repository string to the registry its image would be pulled - * from: a host the string names itself, else a registry declared elsewhere - * in the same document, else Docker Hub. + * from: a host the string names itself, else the registry the caller says + * was declared for it, else Docker Hub. * * `undefined` means the reference is malformed — a name outside the OCI - * grammar, or a `documentRegistry` that is not a host — and never "no + * grammar, or a `declaredRegistry` that is not a host — and never "no * registry was found", because the last step always produces one. That last * step is a guess, which is why the answer carries its * {@link ResolvedReference.source}: a not-found from a registry nobody named * says something about the guess rather than about the image, and only the * source distinguishes the two. */ -function resolveReference(repository: string, documentRegistry: string | undefined): ResolvedReference | undefined { - const reference = selectRegistry(repository, documentRegistry); +function resolveReference(repository: string, declaredRegistry: string | undefined): ResolvedReference | undefined { + const reference = selectRegistry(repository, declaredRegistry); if (reference === undefined || !isDockerHub(reference.host) || reference.name.includes('/')) { return reference; } // Docker's own normalization, applied however the host was arrived at: - // `nginx`, `docker.io/nginx` and a document registry of `docker.io` all + // `nginx`, `docker.io/nginx` and a declared registry of `docker.io` all // address `library/nginx`, and Hub serves the long form only. return { ...reference, name: `${DOCKER_HUB_LIBRARY_NAMESPACE}/${reference.name}` }; } /** Picks which of the three registry sources applies, before Hub name normalization. */ -function selectRegistry(repository: string, documentRegistry: string | undefined): ResolvedReference | undefined { +function selectRegistry(repository: string, declaredRegistry: string | undefined): ResolvedReference | undefined { const explicit = resolveExplicitHost(repository); if (explicit !== undefined) { @@ -98,14 +96,17 @@ function selectRegistry(repository: string, documentRegistry: string | undefined return undefined; } - if (documentRegistry === undefined || documentRegistry === '') { + // An empty string is how a caller reading a half-typed file says "nothing + // declared"; it is not a registry named badly, so it falls back like the + // absence it is. + if (declaredRegistry === undefined || declaredRegistry === '') { return { host: DOCKER_HUB_HOST, name: repository, source: 'docker-hub-fallback' }; } - // A declared registry that is not a host is a malformed document, not an - // invitation to guess Hub instead: falling through would answer a - // question about a registry the document never asked about. - return HOST_PATTERN.test(documentRegistry) ? { host: documentRegistry, name: repository, source: 'document' } : undefined; + // A non-empty declared registry that is not a host is a malformed + // reference, not an invitation to guess Hub instead: falling through would + // answer a question about a registry nobody asked about. + return HOST_PATTERN.test(declaredRegistry) ? { host: declaredRegistry, name: repository, source: 'declared' } : undefined; } export { resolveExplicitHost, resolveReference }; diff --git a/packages/oci-registry/src/verdict.ts b/packages/oci-registry/src/verdict.ts index e2549cf..fdce923 100644 --- a/packages/oci-registry/src/verdict.ts +++ b/packages/oci-registry/src/verdict.ts @@ -7,7 +7,7 @@ * controls that. */ type UnverifiableReason = - /** The repository named no registry and the document declared none, so + /** The repository named no registry and none was declared for it, so * Docker Hub was guessed — and the guess answered not-found. That is no * evidence the image is missing, only that this tool never knew where to * look. A bare internal service name absent from Hub is the ordinary case @@ -31,7 +31,7 @@ type UnverifiableReason = /** The registry responded, but not in a way this checker understands. */ | 'unexpected-response' /** Some part of the reference is outside the grammar it has to satisfy: the - * tag, the repository name, or a registry the document declared. Rejected + * tag, the repository name, or a registry declared for it. Rejected * before any request is issued — building a manifest URL out of an * unvalidated reference is how a crafted values file smuggles a path * traversal, or a different host, into the request. */