fix bug downloading delegated targets (#278)

.changeset/metal-dots-perform.md

@@ -0,0 +1,5 @@
+---
+'tuf-js': patch
+---
+
+Fix error downloading delegated targets

packages/client/src/__tests__/utils/url.ts

@@ -0,0 +1,31 @@
+import * as url from '../../utils/url';
+
+describe('url', () => {
+  describe('join', () => {
+    describe('when url is valid', () => {
+      it('returns the full URL', () => {
+        expect(url.join('https://foo.com', '')).toBe('https://foo.com/');
+        expect(url.join('https://foo.com', 'path')).toBe(
+          'https://foo.com/path'
+        );
+        expect(url.join('https://foo.com/', 'path')).toBe(
+          'https://foo.com/path'
+        );
+        expect(url.join('https://foo.com/', '/path')).toBe(
+          'https://foo.com/path'
+        );
+        expect(url.join('https://foo.com', '/path')).toBe(
+          'https://foo.com/path'
+        );
+      });
+    });
+
+    describe('when url is invalid', () => {
+      it('throws an error', () => {
+        expect(() => url.join('', '')).toThrow('Invalid URL');
+        expect(() => url.join('', 'path')).toThrow('Invalid URL');
+        expect(() => url.join('1.2.3.4', 'path')).toThrow('Invalid URL');
+      });
+    });
+  });
+});

packages/client/src/updater.ts

@@ -10,6 +10,7 @@ import {
 } from './error';
 import { DefaultFetcher, Fetcher } from './fetcher';
 import { TrustedMetadataStore } from './store';
+import * as url from './utils/url';
 
 export interface UpdaterOptions {
   metadataDir: string;
@@ -101,15 +102,16 @@ export class Updater {
 
     if (consistentSnapshot && this.config.prefixTargetsWithHash) {
       const hashes = Object.values(targetInfo.hashes);
-      const basename = path.basename(targetFilePath);
-      targetFilePath = `${hashes[0]}.${basename}`;
+      const { dir, base } = path.parse(targetFilePath);
+      const filename = `${hashes[0]}.${base}`;
+      targetFilePath = dir ? `${dir}/${filename}` : filename;
     }
 
-    const url = path.join(targetBaseUrl, targetFilePath);
+    const targetUrl = url.join(targetBaseUrl, targetFilePath);
 
     // Client workflow 5.7.3: download target file
     await this.fetcher.downloadFile(
-      url,
+      targetUrl,
       targetInfo.length,
       async (fileName) => {
         // Verify hashes and length of downloaded file
@@ -158,11 +160,11 @@ export class Updater {
     const upperBound = lowerBound + this.config.maxRootRotations;
 
     for (let version = lowerBound; version <= upperBound; version++) {
-      const url = path.join(this.metadataBaseUrl, `${version}.root.json`);
+      const rootUrl = url.join(this.metadataBaseUrl, `${version}.root.json`);
       try {
         // Client workflow 5.3.3: download new root metadata file
         const bytesData = await this.fetcher.downloadBytes(
-          url,
+          rootUrl,
           this.config.rootMaxLength
         );
 
@@ -189,11 +191,11 @@ export class Updater {
     }
 
     //Load from remote (whether local load succeeded or not)
-    const url = path.join(this.metadataBaseUrl, `timestamp.json`);
+    const timestampUrl = url.join(this.metadataBaseUrl, 'timestamp.json');
 
     // Client workflow 5.4.1: download timestamp metadata file
     const bytesData = await this.fetcher.downloadBytes(
-      url,
+      timestampUrl,
       this.config.timestampMaxLength
     );
 
@@ -234,14 +236,17 @@ export class Updater {
         ? snapshotMeta.version
         : undefined;
 
-      const url = path.join(
+      const snapshotUrl = url.join(
         this.metadataBaseUrl,
-        version ? `${version}.snapshot.json` : `snapshot.json`
+        version ? `${version}.snapshot.json` : 'snapshot.json'
       );
 
       try {
         // Client workflow 5.5.1: download snapshot metadata file
-        const bytesData = await this.fetcher.downloadBytes(url, maxLength);
+        const bytesData = await this.fetcher.downloadBytes(
+          snapshotUrl,
+          maxLength
+        );
 
         // Client workflow 5.5.2 - 5.5.6
         this.trustedSet.updateSnapshot(bytesData);
@@ -284,14 +289,17 @@ export class Updater {
         ? metaInfo.version
         : undefined;
 
-      const url = path.join(
+      const metadataUrl = url.join(
         this.metadataBaseUrl,
         version ? `${version}.${role}.json` : `${role}.json`
       );
 
       try {
         // Client workflow 5.6.1: download targets metadata file
-        const bytesData = await this.fetcher.downloadBytes(url, maxLength);
+        const bytesData = await this.fetcher.downloadBytes(
+          metadataUrl,
+          maxLength
+        );
 
         // Client workflow 5.6.2 - 5.6.6
         this.trustedSet.updateDelegatedTargets(bytesData, role, parentRole);
@@ -383,7 +391,10 @@ export class Updater {
     if (!this.targetDir) {
       throw new ValueError('Target directory not set');
     }
-    return path.join(this.targetDir, targetInfo.path);
+
+    // URL encode target path
+    const filePath = encodeURIComponent(targetInfo.path);
+    return path.join(this.targetDir, filePath);
   }
 
   private async persistMetadata(metaDataName: string, bytesData: Buffer) {

packages/client/src/utils/url.ts

@@ -0,0 +1,15 @@
+import { URL } from 'url';
+
+export function join(base: string, path: string): string {
+  return new URL(
+    ensureTrailingSlash(base) + removeLeadingSlash(path)
+  ).toString();
+}
+
+function ensureTrailingSlash(path: string): string {
+  return path.endsWith('/') ? path : path + '/';
+}
+
+function removeLeadingSlash(path: string): string {
+  return path.startsWith('/') ? path.slice(1) : path;
+}

scripts/sigstore-tuf-repo-config.json

@@ -1,6 +1,6 @@
 {
-    "targetFiles": "artifact.pub,ctfe.pub,ctfe_2022.pub,fulcio.crt.pem,fulcio_intermediate_v1.crt.pem,fulcio_v1.crt.pem,rekor.pub",
+    "targetFiles": "trusted_root.json,registry.npmjs.org/keys.json",
     "metadataBaseUrl": "https://sigstore-tuf-root.storage.googleapis.com",
     "targetBaseUrl": "https://sigstore-tuf-root.storage.googleapis.com/targets", 
     "rootMetadataUrl": "https://sigstore-tuf-root.storage.googleapis.com/1.root.json" 
-}
+}