Encode slashes in package names in the registry manifest request (#5716)

* Encode slashes in package names in the registry manifest request

Co-Authored-By: Max Brosnahan <1177034+gingermusketeer@users.noreply.github.com>

* Update dummy.registry.ts

* Fix tests

* Add guide for Azure Artifacts

* Update azure-artifacts.md

* Update azure-artifacts.md

* Typo

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
Co-authored-by: Max Brosnahan <1177034+gingermusketeer@users.noreply.github.com>

8 files changed, 100 insertions, 15 deletions

diff --git a/docs/guides/install/azure-artifacts.md b/docs/guides/install/azure-artifacts.md
new file mode 100644
index 000000000..5205e4e13
--- /dev/null
+++ b/docs/guides/install/azure-artifacts.md
@@ -0,0 +1,53 @@
+---
+name: Using bun install with an Azure Artifacts npm registry
+---
+
+[Azure Artifacts](https://azure.microsoft.com/en-us/products/devops/artifacts) is a package management system for Azure DevOps. It allows you to host your own private npm registry, npm packages, and other types of packages as well.
+
+To use it with `bun install`, add a `bunfig.toml` file to your project with the following contents:
+
+### Configure with bunfig.toml
+
+```toml#bunfig.toml
+[install.registry]
+url = "https://pkgs.dev.azure.com/my-azure-artifacts-user/_packaging/my-azure-artifacts-user/npm/registry"
+username = "my-azure-artifacts-user"
+password = "$NPM_PASSWORD"
+```
+
+Make sure to replace `my-azure-artifacts-user` with your Azure Artifacts username, such as `jarred1234`.
+
+Set the `$NPM_PASSWORD` environment variable to your Azure Artifacts npm registry password and Bun will automatically replace it with the correct value. You can also choose not to use an environment variable and instead hardcode your password in the `bunfig.toml` file, but be careful not to commit it to source control.
+
+Note: **password must not be base64 encoded**. In [Azure Artifact's](https://learn.microsoft.com/en-us/azure/devops/artifacts/npm/npmrc?view=azure-devops&tabs=windows%2Cclassic) instructions for `.npmrc`, they say to base64 encode the password. Do not do this for `bun install`. Bun will automatically base64 encode the password for you if needed.
+
+To un-base64 encode a password, you can open your browser console and run:
+
+```js
+atob("base64-encoded-password");
+```
+
+If it ends with `==`, it probably is base64 encoded.
+
+### Configure with environment variables
+
+You can also use an environment variable to configure Azure Artifacts with bun install.
+
+Like with the `npm` CLI, the environment variable to use is `NPM_CONFIG_REGISTRY`.
+
+The URL should include `:username` and `:_password` as query parameters. For example:
+
+```bash
+NPM_CONFIG_REGISTRY=https://pkgs.dev.azure.com/my-azure-artifacts-user/_packaging/my-azure-artifacts-user/npm/registry/:username=my-azure-artifacts-user:_password=my-azure-artifacts-password
+```
+
+Make sure to:
+
+- Replace `my-azure-artifacts-user` with your Azure Artifacts username, such as `jarred1234`
+- Replace `my-azure-artifacts-password` with the non-base64 encoded password for your Azure Artifacts npm registry. If it ends with `==`, it probably is base64 encoded.
+
+To un-base64 encode a password, you can open your browser console and run:
+
+```js
+atob("base64-encoded-password");
+```
diff --git a/src/env_loader.zig b/src/env_loader.zig
index 6957a1a26..213d14ab8 100644
--- a/src/env_loader.zig
+++ b/src/env_loader.zig
@@ -172,7 +172,12 @@ pub const Loader = struct {
     }
 
     pub fn getAuto(this: *const Loader, key: string) string {
-        return this.get(key) orelse key;
+        // If it's "" or "$", it's not a variable
+        if (key.len < 2 or key[0] != '$') {
+            return key;
+        }
+
+        return this.get(key[1..]) orelse key;
     }
 
     /// Load values from the environment into Define.
diff --git a/src/install/install.zig b/src/install/install.zig
index 722d46be1..158130b25 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -252,9 +252,22 @@ const NetworkTask = struct {
         warn_on_error: bool,
     ) !void {
         this.url_buf = blk: {
+
+            // Not all registries support scoped package names when fetching the manifest.
+            // registry.npmjs.org supports both "@storybook%2Faddons" and "@storybook/addons"
+            // Other registries like AWS codeartifact only support the former.
+            // "npm" CLI requests the manifest with the encoded name.
+            var arena = std.heap.ArenaAllocator.init(bun.default_allocator);
+            defer arena.deinit();
+            var stack_fallback_allocator = std.heap.stackFallback(512, arena.allocator());
+            var encoded_name = name;
+            if (strings.containsChar(name, '/')) {
+                encoded_name = try std.mem.replaceOwned(u8, stack_fallback_allocator.get(), name, "/", "%2f");
+            }
+
             const tmp = bun.JSC.URL.join(
                 bun.String.fromUTF8(scope.url.href),
-                bun.String.fromUTF8(name),
+                bun.String.fromUTF8(encoded_name),
             );
             defer tmp.deref();
 
diff --git a/src/install/npm.zig b/src/install/npm.zig
index 9f3f2952c..fec545b0c 100644
--- a/src/install/npm.zig
+++ b/src/install/npm.zig
@@ -68,6 +68,17 @@ pub const Registry = struct {
 
         pub fn fromAPI(name: string, registry_: Api.NpmRegistry, allocator: std.mem.Allocator, env: *DotEnv.Loader) !Scope {
             var registry = registry_;
+
+            // Support $ENV_VAR for registry URLs
+            if (strings.startsWithChar(registry_.url, '$')) {
+                // If it became "$ENV_VAR/", then we need to remove the trailing slash
+                if (env.get(strings.trim(registry_.url[1..], "/"))) |replaced_url| {
+                    if (replaced_url.len > 1) {
+                        registry.url = replaced_url;
+                    }
+                }
+            }
+
             var url = URL.parse(registry.url);
             var auth: string = "";
 
diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts
index 4461584e4..c00c0ba7b 100644
--- a/test/cli/install/bun-add.test.ts
+++ b/test/cli/install/bun-add.test.ts
@@ -232,11 +232,11 @@ it("should handle @scoped names", async () => {
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  expect(err.split(/\r?\n/)).toContain('error: package "@bar/baz" not found localhost/@bar/baz 404');
+  expect(err.split(/\r?\n/)).toContain('error: package "@bar/baz" not found localhost/@bar%2fbaz 404');
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBe("");
   expect(await exited).toBe(1);
-  expect(urls.sort()).toEqual([`${root_url}/@bar/baz`]);
+  expect(urls.sort()).toEqual([`${root_url}/@bar%2fbaz`]);
   expect(requested).toBe(1);
   try {
     await access(join(package_dir, "bun.lockb"));
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index c4accb1b4..25cd0aca7 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -39,7 +39,7 @@ it("should report connection errors", async () => {
     `
 [install]
 cache = false
-registry = "http://localhost:${server.port}/"
+registry = "http://${server.hostname}:${server.port}/"
 `,
   );
   await writeFile(
@@ -62,7 +62,7 @@ registry = "http://localhost:${server.port}/"
   });
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
-  expect(err.split(/\r?\n/)).toContain("error: ConnectionClosed downloading package manifest bar");
+  expect(err.split(/\r?\n/)).toContain("error: ConnectionRefused downloading package manifest bar");
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBeEmpty();
   expect(await exited).toBe(1);
@@ -112,7 +112,7 @@ it("should handle missing package", async () => {
 
 it("should handle @scoped authentication", async () => {
   let seen_token = false;
-  const url = `${root_url}/@foo/bar`;
+  const url = `${root_url}/@foo%2fbar`;
   const urls: string[] = [];
   setHandler(async request => {
     expect(request.method).toBe("GET");
@@ -2197,7 +2197,7 @@ it("should handle unscoped alias on scoped dependency", async () => {
     " 1 packages installed",
   ]);
   expect(await exited).toBe(0);
-  expect(urls.sort()).toEqual([`${root_url}/@barn/moo`, `${root_url}/@barn/moo-0.1.0.tgz`]);
+  expect(urls.sort()).toEqual([`${root_url}/@barn%2fmoo`, `${root_url}/@barn/moo-0.1.0.tgz`]);
   expect(requested).toBe(2);
   expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "@barn", "moo"]);
   expect(await readdirSorted(join(package_dir, "node_modules", "@barn"))).toEqual(["moo"]);
@@ -2327,7 +2327,7 @@ it("should handle aliased dependency with existing lockfile", async () => {
   ]);
   expect(await exited1).toBe(0);
   expect(urls.sort()).toEqual([
-    `${root_url}/@barn/moo`,
+    `${root_url}/@barn%2fmoo`,
     `${root_url}/@barn/moo-0.1.0.tgz`,
     `${root_url}/bar`,
     `${root_url}/bar-0.0.2.tgz`,
diff --git a/test/cli/install/bun-update.test.ts b/test/cli/install/bun-update.test.ts
index 2a6ee4eaf..ff8e22c37 100644
--- a/test/cli/install/bun-update.test.ts
+++ b/test/cli/install/bun-update.test.ts
@@ -185,7 +185,7 @@ it("should update to latest versions of dependencies", async () => {
   ]);
   expect(await exited1).toBe(0);
   expect(urls.sort()).toEqual([
-    `${root_url}/@barn/moo`,
+    `${root_url}/@barn%2fmoo`,
     `${root_url}/@barn/moo-0.1.0.tgz`,
     `${root_url}/baz`,
     `${root_url}/baz-0.0.3.tgz`,
@@ -236,7 +236,7 @@ it("should update to latest versions of dependencies", async () => {
   ]);
   expect(await exited2).toBe(0);
   expect(urls.sort()).toEqual([
-    `${root_url}/@barn/moo`,
+    `${root_url}/@barn%2fmoo`,
     `${root_url}/@barn/moo-0.1.0.tgz`,
     `${root_url}/baz`,
     `${root_url}/baz-0.0.5.tgz`,
diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts
index 69af0c381..5d0982f57 100644
--- a/test/cli/install/dummy.registry.ts
+++ b/test/cli/install/dummy.registry.ts
@@ -34,16 +34,19 @@ export let root_url: string;
 export function dummyRegistry(urls: string[], info: any = { "0.0.2": {} }) {
   const _handler: Handler = async request => {
     urls.push(request.url);
+    const url = request.url.replaceAll("%2f", "/");
+
     expect(request.method).toBe("GET");
-    if (request.url.endsWith(".tgz")) {
-      return new Response(file(join(import.meta.dir, basename(request.url).toLowerCase())));
+    if (url.endsWith(".tgz")) {
+      return new Response(file(join(import.meta.dir, basename(url).toLowerCase())));
     }
     expect(request.headers.get("accept")).toBe(
       "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*",
     );
     expect(request.headers.get("npm-auth-type")).toBe(null);
     expect(await request.text()).toBe("");
-    const name = request.url.slice(request.url.indexOf("/", root_url.length) + 1);
+
+    const name = url.slice(url.indexOf("/", root_url.length) + 1);
     const versions: Record<string, Pkg> = {};
     let version;
     for (version in info) {
@@ -52,7 +55,7 @@ export function dummyRegistry(urls: string[], info: any = { "0.0.2": {} }) {
         name,
         version,
         dist: {
-          tarball: `${request.url}-${info[version].as ?? version}.tgz`,
+          tarball: `${url}-${info[version].as ?? version}.tgz`,
         },
         ...info[version],
       };