diff --git a/src/spec-configuration/containerCollectionsOCI.ts b/src/spec-configuration/containerCollectionsOCI.ts index 7f95785d0..9a23f0e8a 100644 --- a/src/spec-configuration/containerCollectionsOCI.ts +++ b/src/spec-configuration/containerCollectionsOCI.ts @@ -58,13 +58,38 @@ interface OCITagList { tags: string[]; } -export function getRef(output: Log, resourceAndVersion: string): OCIRef { - - // ex: ghcr.io/codspace/features/ruby:1 - // ex: ghcr.io/codspace/templates/ruby:1 - const splitOnColon = resourceAndVersion.split(':'); - const resource = splitOnColon[0]; - const version = splitOnColon[1] ? splitOnColon[1] : 'latest'; +// Following Spec: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests +// Alternative Spec: https://docs.docker.com/registry/spec/api/#overview +// +// Entire path ('namespace' in spec terminology) for the given repository +// (eg: devcontainers/features/go) +const regexForPath = /^[a-z0-9]+([._-][a-z0-9]+)*(\/[a-z0-9]+([._-][a-z0-9]+)*)*$/; +// MUST be either (a) the digest of the manifest or (b) a tag +// MUST be at most 128 characters in length and MUST match the following regular expression: +const regexForReference = /^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$/; + +// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests +// Attempts to parse the given string into an OCIRef +export function getRef(output: Log, input: string): OCIRef | undefined { + // Normalize input by downcasing entire string + input = input.toLowerCase(); + + const indexOfLastColon = input.lastIndexOf(':'); + + let resource = ''; + let version = ''; // TODO: Support parsing out manifest digest (...@sha256:...) + + // 'If' condition is true in the following cases: + // 1. The final colon is before the first slash (a port) : eg: ghcr.io:8081/codspace/features/ruby + // 2. There is no version : eg: ghcr.io/codspace/features/ruby + // In both cases, assume 'latest' tag. + if (indexOfLastColon === -1 || indexOfLastColon < input.indexOf('/')) { + resource = input; + version = 'latest'; + } else { + resource = input.substring(0, indexOfLastColon); + version = input.substring(indexOfLastColon + 1); + } const splitOnSlash = resource.split('/'); @@ -79,10 +104,22 @@ export function getRef(output: Log, resourceAndVersion: string): OCIRef { output.write(`id: ${id}`, LogLevel.Trace); output.write(`version: ${version}`, LogLevel.Trace); output.write(`owner: ${owner}`, LogLevel.Trace); - output.write(`namespace: ${namespace}`, LogLevel.Trace); + output.write(`namespace: ${namespace}`, LogLevel.Trace); // TODO: We assume 'namespace' includes at least one slash (eg: 'devcontainers/features') output.write(`registry: ${registry}`, LogLevel.Trace); output.write(`path: ${path}`, LogLevel.Trace); + // Validate results of parse. + + if (!regexForPath.exec(path)) { + output.write(`Parsed path '${path}' for input '${input}' failed validation.`, LogLevel.Error); + return undefined; + } + + if (!regexForReference.test(version)) { + output.write(`Parsed version '${version}' for input '${input}' failed validation.`, LogLevel.Error); + return undefined; + } + return { id, version, @@ -229,7 +266,7 @@ export async function getPublishedVersions(ref: OCIRef, output: Log, sorted: boo let authToken = await fetchRegistryAuthToken(output, ref.registry, ref.path, process.env, 'pull'); if (!authToken) { - output.write(`(!) ERR: Failed to publish ${collectionType}: ${ref.resource}`, LogLevel.Error); + output.write(`(!) ERR: Failed to get published versions for ${collectionType}: ${ref.resource}`, LogLevel.Error); return undefined; } diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 873b87027..906ec6d52 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -12,7 +12,7 @@ import { mkdirpLocal, readLocalFile, rmLocal, writeLocalFile, cpDirectoryLocal, import { Log, LogLevel } from '../spec-utils/log'; import { request } from '../spec-utils/httpRequest'; import { computeFeatureInstallationOrder } from './containerFeaturesOrder'; -import { fetchOCIFeature, getOCIFeatureSet, fetchOCIFeatureManifestIfExistsFromUserIdentifier } from './containerFeaturesOCI'; +import { fetchOCIFeature, tryGetOCIFeatureSet, fetchOCIFeatureManifestIfExistsFromUserIdentifier } from './containerFeaturesOCI'; import { OCIManifest, OCIRef } from './containerCollectionsOCI'; // v1 @@ -769,8 +769,7 @@ export async function processFeatureIdentifier(output: Log, configPath: string, // (6) Oci Identifier if (type === 'oci' && manifest) { - let newFeaturesSet: FeatureSet = getOCIFeatureSet(output, userFeature.id, userFeature.options, manifest, originalUserFeatureId); - return newFeaturesSet; + return tryGetOCIFeatureSet(output, userFeature.id, userFeature.options, manifest, originalUserFeatureId); } output.write(`Github feature.`); diff --git a/src/spec-configuration/containerFeaturesOCI.ts b/src/spec-configuration/containerFeaturesOCI.ts index f6117b280..91bc1c5f9 100644 --- a/src/spec-configuration/containerFeaturesOCI.ts +++ b/src/spec-configuration/containerFeaturesOCI.ts @@ -2,9 +2,12 @@ import { Log, LogLevel } from '../spec-utils/log'; import { Feature, FeatureSet } from './containerFeaturesConfiguration'; import { fetchOCIManifestIfExists, getBlob, getRef, OCIManifest } from './containerCollectionsOCI'; -export function getOCIFeatureSet(output: Log, identifier: string, options: boolean | string | Record, manifest: OCIManifest, originalUserFeatureId: string): FeatureSet { - +export function tryGetOCIFeatureSet(output: Log, identifier: string, options: boolean | string | Record, manifest: OCIManifest, originalUserFeatureId: string): FeatureSet | undefined { const featureRef = getRef(output, identifier); + if (!featureRef) { + output.write(`Unable to parse '${identifier}'`, LogLevel.Error); + return undefined; + } const feat: Feature = { id: featureRef.id, @@ -30,6 +33,9 @@ export function getOCIFeatureSet(output: Log, identifier: string, options: boole export async function fetchOCIFeatureManifestIfExistsFromUserIdentifier(output: Log, env: NodeJS.ProcessEnv, identifier: string, manifestDigest?: string, authToken?: string): Promise { const featureRef = getRef(output, identifier); + if (!featureRef) { + return undefined; + } return await fetchOCIManifestIfExists(output, env, featureRef, manifestDigest, authToken); } diff --git a/src/spec-configuration/containerTemplatesOCI.ts b/src/spec-configuration/containerTemplatesOCI.ts index 84a332400..98a9f93c9 100644 --- a/src/spec-configuration/containerTemplatesOCI.ts +++ b/src/spec-configuration/containerTemplatesOCI.ts @@ -122,6 +122,9 @@ export async function fetchTemplate(output: Log, selectedTemplate: SelectedTempl async function fetchOCITemplateManifestIfExistsFromUserIdentifier(output: Log, env: NodeJS.ProcessEnv, identifier: string, manifestDigest?: string, authToken?: string): Promise { const templateRef = getRef(output, identifier); + if (!templateRef) { + return undefined; + } return await fetchOCIManifestIfExists(output, env, templateRef, manifestDigest, authToken); } diff --git a/src/spec-node/collectionCommonUtils/publishCommandImpl.ts b/src/spec-node/collectionCommonUtils/publishCommandImpl.ts index e5d3bbf61..2976f837f 100644 --- a/src/spec-node/collectionCommonUtils/publishCommandImpl.ts +++ b/src/spec-node/collectionCommonUtils/publishCommandImpl.ts @@ -44,7 +44,7 @@ export async function doPublishCommand(version: string, ociRef: OCIRef, outputDi const publishedVersions = await getPublishedVersions(ociRef, output); if (!publishedVersions) { - process.exit(1); + return false; } const semanticVersions: string[] | undefined = getSermanticVersions(version, publishedVersions, output); @@ -53,11 +53,12 @@ export async function doPublishCommand(version: string, ociRef: OCIRef, outputDi output.write(`Publishing versions: ${semanticVersions.toString()}...`, LogLevel.Info); const pathToTgz = path.join(outputDir, getArchiveName(ociRef.id, collectionType)); if (! await pushOCIFeatureOrTemplate(output, ociRef, pathToTgz, semanticVersions, collectionType)) { - output.write(`(!) ERR: Failed to publish ${collectionType}: ${ociRef.resource}`, LogLevel.Error); - process.exit(1); + output.write(`(!) ERR: Failed to publish ${collectionType}: '${ociRef.resource}'`, LogLevel.Error); + return false; } - output.write(`Published ${collectionType}: ${ociRef.id}...`, LogLevel.Info); } + output.write(`Published ${collectionType}: ${ociRef.id}...`, LogLevel.Info); + return true; } export async function doPublishMetadata(collectionRef: OCICollectionRef, outputDir: string, output: Log, collectionType: string) { @@ -67,7 +68,8 @@ export async function doPublishMetadata(collectionRef: OCICollectionRef, outputD const pathToCollectionFile = path.join(outputDir, OCICollectionFileName); if (! await pushCollectionMetadata(output, collectionRef, pathToCollectionFile, collectionType)) { output.write(`(!) ERR: Failed to publish collection metadata: ${OCICollectionFileName}`, LogLevel.Error); - process.exit(1); + return false; } output.write('Published collection metadata...', LogLevel.Info); + return true; } diff --git a/src/spec-node/featuresCLI/info.ts b/src/spec-node/featuresCLI/info.ts index 23a911e4c..49681e2db 100644 --- a/src/spec-node/featuresCLI/info.ts +++ b/src/spec-node/featuresCLI/info.ts @@ -40,6 +40,14 @@ async function featuresInfo({ }, pkg, new Date(), disposables, true); const featureOciRef = getRef(output, featureId); + if (!featureOciRef) { + if (outputFormat === 'json') { + output.raw(JSON.stringify({}), LogLevel.Info); + } else { + output.raw(`Failed to parse Feature identifier '${featureId}'\n`, LogLevel.Error); + } + process.exit(1); + } const publishedVersions = await getPublishedVersions(featureOciRef, output, true); if (!publishedVersions || publishedVersions.length === 0) { diff --git a/src/spec-node/featuresCLI/publish.ts b/src/spec-node/featuresCLI/publish.ts index 249f9d6c8..34027f133 100644 --- a/src/spec-node/featuresCLI/publish.ts +++ b/src/spec-node/featuresCLI/publish.ts @@ -77,7 +77,15 @@ async function featuresPublish({ const resource = `${registry}/${namespace}/${f.id}`; const featureRef = getRef(output, resource); - await doPublishCommand(f.version, featureRef, outputDir, output, collectionType); + if (!featureRef) { + output.write(`(!) Could not parse provided Feature identifier: '${resource}'`, LogLevel.Error); + process.exit(1); + } + + if (! await doPublishCommand(f.version, featureRef, outputDir, output, collectionType)) { + output.write(`(!) ERR: Failed to publish '${resource}'`, LogLevel.Error); + process.exit(1); + } } const featureCollectionRef: OCICollectionRef = { @@ -86,7 +94,10 @@ async function featuresPublish({ version: 'latest' }; - await doPublishMetadata(featureCollectionRef, outputDir, output, collectionType); + if (! await doPublishMetadata(featureCollectionRef, outputDir, output, collectionType)) { + output.write(`(!) ERR: Failed to publish '${featureCollectionRef.registry}/${featureCollectionRef.path}'`, LogLevel.Error); + process.exit(1); + } // Cleanup await rmLocal(outputDir, { recursive: true, force: true }); diff --git a/src/spec-node/templatesCLI/publish.ts b/src/spec-node/templatesCLI/publish.ts index e876f1fb8..94bd90788 100644 --- a/src/spec-node/templatesCLI/publish.ts +++ b/src/spec-node/templatesCLI/publish.ts @@ -78,6 +78,11 @@ async function templatesPublish({ const resource = `${registry}/${namespace}/${t.id}`; const templateRef = getRef(output, resource); + if (!templateRef) { + output.write(`(!) Could not parse provided Template identifier: '${resource}'`, LogLevel.Error); + process.exit(1); + } + await doPublishCommand(t.version, templateRef, outputDir, output, collectionType); } diff --git a/src/spec-node/utils.ts b/src/spec-node/utils.ts index aa81a09b8..d0814884b 100644 --- a/src/spec-node/utils.ts +++ b/src/spec-node/utils.ts @@ -215,6 +215,9 @@ export async function inspectDockerImage(params: DockerResolverParameters | Dock export async function inspectImageInRegistry(output: Log, name: string, authToken?: string): Promise { const resourceAndVersion = qualifyImageName(name); const ref = getRef(output, resourceAndVersion); + if (!ref) { + throw new Error(`Could not parse image name '${name}'`); + } const auth = authToken ?? await fetchRegistryAuthToken(output, ref.registry, ref.path, process.env, 'pull'); const registryServer = ref.registry === 'docker.io' ? 'registry-1.docker.io' : ref.registry; diff --git a/src/test/container-features/containerFeaturesOCI.test.ts b/src/test/container-features/containerFeaturesOCI.test.ts index f47bac425..fafcdfb8d 100644 --- a/src/test/container-features/containerFeaturesOCI.test.ts +++ b/src/test/container-features/containerFeaturesOCI.test.ts @@ -4,9 +4,122 @@ import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log'; export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace)); +describe('getRef()', async function () { + this.timeout('120s'); + + it('valid getRef() with a tag', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker:latest'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + }); + + it('valid getRef() without a version tag', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() automatically downcases', async () => { + const feat = getRef(output, 'ghcr.io/DeVContainERS/templates/Docker-FROM-Docker'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'ghcr.io'); + assert.equal(feat.resource, 'ghcr.io/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() with a registry that contains a port.', async () => { + const feat = getRef(output, 'docker.io:8001/devcontainers/templates/docker-from-docker:latest'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'docker-from-docker'); + assert.equal(feat.namespace, 'devcontainers/templates'); + assert.equal(feat.owner, 'devcontainers'); + assert.equal(feat.registry, 'docker.io:8001'); + assert.equal(feat.resource, 'docker.io:8001/devcontainers/templates/docker-from-docker'); + assert.equal(feat.path, 'devcontainers/templates/docker-from-docker'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('valid getRef() really short path and no version', async () => { + const feat = getRef(output, 'docker.io:8001/a/b/c'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } + assert.ok(feat); + assert.equal(feat.id, 'c'); + assert.equal(feat.namespace, 'a/b'); + assert.equal(feat.owner, 'a'); + assert.equal(feat.registry, 'docker.io:8001'); + assert.equal(feat.resource, 'docker.io:8001/a/b/c'); + assert.equal(feat.path, 'a/b/c'); + assert.equal(feat.version, 'latest'); // Defaults to 'latest' if not version supplied. + }); + + it('invalid getRef() with duplicate version tags', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from-docker:latest:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with invalid character in namespace', async () => { + const feat = getRef(output, 'ghcr.io/devco%ntainers/templates/docker-from-docker:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with invalid character in feature name', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers/templates/docker-from@docker:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with missing path with version tag', async () => { + const feat = getRef(output, 'ghcr.io/:latest'); + assert.isUndefined(feat); + }); + + it('invalid getRef() with missing path without version tag', async () => { + const feat = getRef(output, 'ghcr.io'); + assert.isUndefined(feat); + }); + + it('invalid getRef() multiple slashes in sequence', async () => { + const feat = getRef(output, 'ghcr.io/devcontainers//templates/docker-from-docker:latest'); + assert.isUndefined(feat); + }); + +}); + describe('Test OCI Pull', () => { it('Parse OCI identifier', async () => { const feat = getRef(output, 'ghcr.io/codspace/features/ruby:1'); + if (!feat) { + assert.fail('featureRef should not be undefined'); + } output.write(`feat: ${JSON.stringify(feat)}`); assert.equal(feat.id, 'ruby'); @@ -20,6 +133,9 @@ describe('Test OCI Pull', () => { it('Get a manifest by tag', async () => { const featureRef = getRef(output, 'ghcr.io/codspace/features/ruby:1.0.13'); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const manifest = await getManifest(output, process.env, 'https://ghcr.io/v2/codspace/features/ruby/manifests/1.0.13', featureRef); assert.isNotNull(manifest); assert.exists(manifest); @@ -42,6 +158,9 @@ describe('Test OCI Pull', () => { it('Download a feature', async () => { const featureRef = getRef(output, 'ghcr.io/codspace/features/ruby:1.0.13'); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const blobResult = await getBlob(output, process.env, 'https://ghcr.io/v2/codspace/features/ruby/blobs/sha256:8f59630bd1ba6d9e78b485233a0280530b3d0a44338f472206090412ffbd3efb', '/tmp', '/tmp/featureTest', featureRef); assert.isDefined(blobResult); assert.isArray(blobResult?.files); diff --git a/src/test/container-features/containerFeaturesOCIPush.test.ts b/src/test/container-features/containerFeaturesOCIPush.test.ts index 32ebe28ec..2ccfdec7e 100644 --- a/src/test/container-features/containerFeaturesOCIPush.test.ts +++ b/src/test/container-features/containerFeaturesOCIPush.test.ts @@ -46,6 +46,9 @@ describe('Test OCI Push', () => { it('Can check whether a blob exists', async () => { const ociFeatureRef = getRef(output, 'ghcr.io/codspace/features/go:1'); + if (!ociFeatureRef) { + assert.fail('getRef() for the Feature should not be undefined'); + } const { registry, resource } = ociFeatureRef; const sessionAuth = await fetchRegistryAuthToken(output, registry, resource, process.env, 'pull'); if (!sessionAuth) { diff --git a/src/test/container-features/featuresCLICommands.test.ts b/src/test/container-features/featuresCLICommands.test.ts index f265280fc..c25cc85e7 100644 --- a/src/test/container-features/featuresCLICommands.test.ts +++ b/src/test/container-features/featuresCLICommands.test.ts @@ -478,6 +478,9 @@ describe('test function getPublishedVersions', async () => { it('should list published versions', async () => { const resource = 'ghcr.io/devcontainers/features/node'; const featureRef = getRef(output, resource); + if (!featureRef) { + assert.fail('featureRef should not be undefined'); + } const versionsList = await getPublishedVersions(featureRef, output) ?? []; assert.includeMembers(versionsList, ['1', '1.0', '1.0.0', 'latest']); });