fix(docker): reject a shard DSN holding a line break, and scope the loop's shell options

One DSN per line is the protocol between the script and the entrypoint, and the
URL parser strips ASCII line breaks, so a DSN holding one would split into two
bogus DSNs with nothing upstream to reject it.

The loop now runs in a subshell, so its IFS and noglob changes need no restore
and cannot leak into the rest of the entrypoint.

Author: d-cs

apps/webapp/test/runOpsShardDsns.test.ts

@@ -61,3 +61,17 @@ describe("shardMigrationDsns", () => {
     expect(() => shardMigrationDsns('["postgres://h/a"]')).toThrow(/not an object/i);
   });
 });
+
+describe("shardMigrationDsns line protocol", () => {
+  // One DSN per line is the protocol with entrypoint.sh, so a line break would split one DSN into
+  // two bogus ones. The URL parser strips ASCII line breaks, so nothing upstream rejects this.
+  it("throws when a DSN holds a line break", () => {
+    const bad = { key: "a", region: "r", url: "postgres://h/a\npostgres://evil/db" };
+    expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i);
+  });
+
+  it("throws when a directUrl holds a carriage return", () => {
+    const bad = { key: "a", region: "r", url: "postgres://h/a", directUrl: "postgres://h/a\rx" };
+    expect(() => shardMigrationDsns(JSON.stringify([bad]))).toThrow(/line break/i);
+  });
+});

docker/scripts/entrypoint.sh

@@ -65,18 +65,18 @@ if [ -n "$RUN_OPS_SHARDS" ]; then
     shard_dsns=$(node scripts/runOpsShardDsns.mjs)
     # A `for` loop and NOT `... | while read`: a pipeline subshell would swallow a failed migration
     # on any iteration but the last. Here `set -e` stops the boot on the first shard that fails.
-    # IFS is newline-only so a DSN is never split on other whitespace, and `set -f` stops a DSN
-    # query string (it holds `?`) from being read as a glob pattern.
-    old_ifs=$IFS
-    IFS='
+    # The whole loop runs in a subshell, so the IFS and `set -f` changes need no restore and cannot
+    # leak into the rest of the entrypoint. IFS is newline-only so a DSN is never split on other
+    # whitespace, and `set -f` stops a DSN query string (it holds `?`) from acting as a glob.
+    (
+      IFS='
 '
-    set -f
-    for shard_dsn in $shard_dsns; do
-      # Subshell with tracing off so `set -x` does not print the DSN (with credentials) to the logs.
-      (set +x; RUN_OPS_DATABASE_URL="$shard_dsn" DIRECT_URL="$shard_dsn" pnpm --filter @internal/run-ops-database db:migrate:deploy)
-    done
-    set +f
-    IFS=$old_ifs
+      set -f
+      for shard_dsn in $shard_dsns; do
+        # Tracing stays off so `set -x` never prints the DSN (with credentials) to the logs.
+        RUN_OPS_DATABASE_URL="$shard_dsn" DIRECT_URL="$shard_dsn" pnpm --filter @internal/run-ops-database db:migrate:deploy
+      done
+    )
     set -x
     echo "Run-ops shard migrations done"
   else

docker/scripts/runOpsShardDsns.mjs

@@ -39,6 +39,11 @@ export function shardMigrationDsns(raw) {
     if (typeof dsn !== "string" || dsn === "") {
       continue;
     }
+    // One DSN per line IS the protocol with the caller, so a DSN holding a line break would split
+    // into two bogus DSNs. The URL parser strips ASCII line breaks, so nothing upstream rejects it.
+    if (/[\r\n]/.test(dsn)) {
+      throw new Error("RUN_OPS_SHARDS holds a DSN containing a line break");
+    }
     dsns.push(dsn);
   }
   return dsns;