fix(api): mask build arguments and preserve secrets on round trips

apps/api/src/lib/build-arg-fingerprint.ts

@@ -0,0 +1,31 @@
+import { createHmac } from "node:crypto";
+import { isMaskedValue } from "@repo/core";
+import { env } from "../config/env";
+
+/** Compare stored literal args across a write response and deployment history
+ * without returning their values or an offline-guessable unkeyed hash. Scope to
+ * the project, service name and key so writing guesses in another project or
+ * service cannot be used as a fingerprint oracle. */
+export function fingerprintBuildArgs(
+  projectId: string,
+  serviceName: string,
+  args: Record<string, string | null>,
+  templateKeys?: string[],
+): Record<string, string> {
+  const fingerprints: Record<string, string> = {};
+  for (const [key, value] of Object.entries(args)) {
+    // Null inherits a value at build time. Templates also depend on that build's
+    // environment; fingerprinting the expression would falsely attest a value.
+    if (
+      value === null ||
+      isMaskedValue(value) ||
+      (templateKeys ? templateKeys.includes(key) : value.includes("$"))
+    ) {
+      continue;
+    }
+    fingerprints[key] = `hmac-sha256:${createHmac("sha256", env.BETTER_AUTH_SECRET)
+      .update(JSON.stringify(["openship-build-arg-v1", projectId, serviceName, key, value]))
+      .digest("hex")}`;
+  }
+  return fingerprints;
+}

apps/api/src/lib/secret-env.ts

@@ -1,8 +1,8 @@
 /**
- * Compose-service `environment` masking (#336).
+ * Compose-service `environment` and `buildArgs` masking (#336, #854).
  *
- * A compose service's `environment` map is BOTH the deploy spec (injected into
- * the container) AND display data. It routinely holds secrets (DB passwords, API
+ * A compose service's env and build-arg maps are BOTH the deploy spec AND
+ * display data. They routinely hold secrets (DB passwords, API
  * tokens), yet — unlike project env vars, which carry an explicit `isSecret`
  * flag — it's a flat `Record<string,string>` with no secret marker. So instead
  * of a fragile key-name heuristic we mask *every* value on output and offer an
@@ -22,6 +22,7 @@
 // The mask sentinel + predicate live in @repo/core so the dashboard's env editor
 // shares the exact same string (the reveal/round-trip contract depends on it).
 import { ENV_MASK, isMaskedValue } from "@repo/core";
+import { fingerprintBuildArgs } from "./build-arg-fingerprint";
 export { ENV_MASK, isMaskedValue };
 
 /**
@@ -36,6 +37,32 @@ export function maskEnv(env: Record<string, string> | null | undefined): Record<
   return out;
 }
 
+/** Null build args inherit from the build environment; empty strings stay empty. */
+export function maskBuildArgs(args: Record<string, string | null> | null | undefined) {
+  return Object.fromEntries(
+    Object.entries(args ?? {}).map(([key, value]) => [
+      key,
+      value === null ? null : maskValue(value),
+    ]),
+  );
+}
+
+/** Whole-map replacement, like buildArgs before masking, with sentinel recovery. */
+export function unmaskBuildArgs(
+  incoming: Record<string, string | null> | null | undefined,
+  stored: Record<string, string | null> | null | undefined,
+): Record<string, string | null> {
+  const result: Record<string, string | null> = {};
+  for (const [key, value] of Object.entries(incoming ?? {})) {
+    if (isMaskedValue(value)) {
+      if (stored && Object.hasOwn(stored, key)) result[key] = stored[key];
+    } else {
+      result[key] = value;
+    }
+  }
+  return result;
+}
+
 /**
  * An EMPTY value stays empty — there is nothing there to hide, and dots in its
  * place are an active lie: the wizard reads "no value" off the empty string to
@@ -128,31 +155,40 @@ export function mergeServiceEnv(
 }
 
 /** Whether an env map contains any mask sentinel (i.e. an un-revealed value). */
-export function hasMaskedValue(env: Record<string, string> | null | undefined): boolean {
+export function hasMaskedValue(env: Record<string, string | null> | null | undefined): boolean {
   if (!env) return false;
   return Object.values(env).some(isMaskedValue);
 }
 
 /**
- * Mask the `environment` field of a single compose/deployable service. Returns a
+ * Mask the env and build-arg fields of a single compose/deployable service. Returns a
  * shallow copy — the caller's stored object is left untouched. It also removes
  * server-owned interpolation provenance before the service crosses an API
  * boundary, even when the service has no runtime environment map.
  */
 export function maskServiceEnv<
   T extends {
+    name?: string;
+    projectId?: string;
+    buildArgs?: Record<string, string | null> | null;
+    importedSpec?: unknown;
+    driftSpec?: unknown;
     environment?: Record<string, string> | null;
     environmentTemplates?: Record<string, string> | null;
     advanced?: {
       imageTemplate?: unknown;
       environmentTemplateKeys?: string[];
+      buildArgTemplateKeys?: string[];
       [key: string]: unknown;
     } | null;
   },
->(svc: T | null | undefined): T | null | undefined {
+>(svc: T | null | undefined, projectId?: string): T | null | undefined {
   if (!svc) return svc;
   if (
     !svc.environment &&
+    !svc.buildArgs &&
+    !svc.importedSpec &&
+    !svc.driftSpec &&
     !svc.environmentTemplates &&
     !svc.advanced?.imageTemplate &&
     !svc.advanced?.environmentTemplateKeys
@@ -162,7 +198,12 @@ export function maskServiceEnv<
   // `environmentTemplates` is transient parser provenance. Its expressions can
   // contain literal defaults, so never serialize it even though the persisted
   // raw copy is already protected by blanket environment masking.
-  const { environmentTemplates: _templates, ...publicService } = svc;
+  const {
+    environmentTemplates: _templates,
+    importedSpec: _importedSpec,
+    driftSpec: _driftSpec,
+    ...publicService
+  } = svc;
   const advanced = svc.advanced ? { ...svc.advanced } : svc.advanced;
   if (advanced) {
     // Parser provenance is server-owned. Besides preventing a client from
@@ -174,6 +215,17 @@ export function maskServiceEnv<
   return {
     ...publicService,
     ...(svc.environment ? { environment: maskEnv(svc.environment) } : {}),
+    ...(svc.buildArgs ? { buildArgs: maskBuildArgs(svc.buildArgs) } : {}),
+    ...(svc.buildArgs && (projectId || svc.projectId) && svc.name
+      ? {
+          buildArgsFingerprints: fingerprintBuildArgs(
+            (projectId || svc.projectId)!,
+            svc.name,
+            svc.buildArgs,
+            svc.advanced?.buildArgTemplateKeys,
+          ),
+        }
+      : {}),
     ...(advanced !== undefined ? { advanced } : {}),
   } as T;
 }
@@ -185,10 +237,10 @@ export function maskServicesEnv<
     environmentTemplates?: Record<string, string> | null;
     advanced?: { environmentTemplateKeys?: string[]; [key: string]: unknown } | null;
   },
->(svcs: T[] | null | undefined): T[] {
+>(svcs: T[] | null | undefined, projectId?: string): T[] {
   if (!svcs) return [];
   // Elements are concrete services, so the masked result is never null/undefined.
-  return svcs.map((s) => maskServiceEnv(s) as T);
+  return svcs.map((s) => maskServiceEnv(s, projectId) as T);
 }
 
 /** The value-bearing fields of a compose `environmentMeta` entry. */
@@ -256,7 +308,7 @@ export function maskScanService<
 
 /**
  * Mask the compose-service env carried in a deployment's `meta` snapshot
- * (`meta.composeServices[].environment`). Returns a copy — the stored row/meta
+ * (`meta.composeServices[].environment` and `buildArgs`). Returns a copy — the stored row/meta
  * is untouched (rollback/redeploy read the real values back). Apply at the
  * CONTROLLER boundary only: `getDeployment` is also used internally and must
  * keep plaintext. No-op when there's no `meta.composeServices`.
@@ -279,6 +331,7 @@ export function maskDeploymentEnv<T extends { meta?: unknown } | null | undefine
       ...meta,
       composeServices: maskServicesEnv(
         meta.composeServices as { environment?: Record<string, string> | null }[],
+        (dep as { projectId?: string }).projectId,
       ),
     },
   };
@@ -301,6 +354,13 @@ export function maskDriftChanges<T extends { field: string; from: unknown; to: u
         to: maskEnv(c.to as Record<string, string> | null),
       };
     }
+    if (c.field === "buildArgs") {
+      return {
+        ...c,
+        from: maskBuildArgs(c.from as Record<string, string | null> | null),
+        to: maskBuildArgs(c.to as Record<string, string | null> | null),
+      };
+    }
     if (c.field === "advanced") {
       const maskImageTemplate = (value: unknown): unknown => {
         if (!value || typeof value !== "object" || Array.isArray(value)) return value;

apps/api/src/modules/deployments/build-status.service.ts

@@ -184,6 +184,7 @@ export async function getBuildSessionStatus(deploymentId: string) {
           // values on the way back in).
           composeServices: maskServicesEnv(
             (snapshot?.composeServices ?? []).filter((s) => serviceKind(s) === "compose"),
+            project.id,
           ),
         }
       : {};

apps/api/src/modules/deployments/build.service.ts

@@ -59,7 +59,7 @@ import {
 } from "./prepare.service";
 import { ComposeConfigurationError } from "./compose-configuration-error";
 import { getFolderSession } from "../projects/folder/session-store";
-import { hasMaskedValue, isMaskedValue, unmaskEnv } from "../../lib/secret-env";
+import { hasMaskedValue, isMaskedValue, unmaskEnv, unmaskBuildArgs } from "../../lib/secret-env";
 import { assertValidCustomDomains, customHostnamesOf } from "../../lib/custom-domain-guard";
 import {
   assertBuildMinutesAvailable,
@@ -1676,19 +1676,28 @@ export async function requestBuildAccess(
   // captured pre-mask) and the stored service rows — which reconcileComposeSource
   // above just refreshed from a git repo's compose, so this also covers a git
   // first-deploy. A revealed-and-edited value arrives real and passes through.
-  if (effectiveServices?.length && effectiveServices.some((s) => hasMaskedValue(s.environment))) {
+  if (
+    effectiveServices?.some((s) => hasMaskedValue(s.environment) || hasMaskedValue(s.buildArgs))
+  ) {
     const realEnvByName = new Map<string, Record<string, string>>();
+    const realArgsByName = new Map<string, Record<string, string | null>>();
     for (const s of await listProjectComposeServices(project.id)) {
       realEnvByName.set(s.name, (s.environment as Record<string, string> | null) ?? {});
+      realArgsByName.set(s.name, s.buildArgs ?? {});
     }
     for (const s of uploadSession?.services ?? []) {
       if (s.name && s.environment) realEnvByName.set(s.name, s.environment);
+      if (s.name && s.buildArgs) realArgsByName.set(s.name, s.buildArgs);
     }
-    effectiveServices = effectiveServices.map((s) =>
-      s.environment && hasMaskedValue(s.environment)
-        ? { ...s, environment: unmaskEnv(s.environment, realEnvByName.get(s.name) ?? null) }
-        : s,
-    );
+    effectiveServices = effectiveServices.map((s) => ({
+      ...s,
+      ...(hasMaskedValue(s.environment) && {
+        environment: unmaskEnv(s.environment, realEnvByName.get(s.name)),
+      }),
+      ...(hasMaskedValue(s.buildArgs) && {
+        buildArgs: unmaskBuildArgs(s.buildArgs, realArgsByName.get(s.name)),
+      }),
+    }));
   }
 
   const projectDomains = await listProjectRouteRows(project.id);

apps/api/src/modules/services/service.service.ts

@@ -41,10 +41,12 @@ import { encrypt, decrypt } from "../../lib/encryption";
 import {
   ENV_MASK,
   hasMaskedValue,
+  isMaskedValue,
   maskDriftChanges,
   maskServiceEnv,
   mergeServiceEnv,
   unmaskEnv,
+  unmaskBuildArgs,
 } from "../../lib/secret-env";
 import {
   assertNotControlPlane,
@@ -673,7 +675,7 @@ export async function createService(
     image: trimOrNull(data.image),
     build: trimOrNull(data.build),
     dockerfile: trimOrNull(data.dockerfile),
-    buildArgs: data.buildArgs ?? {},
+    buildArgs: unmaskBuildArgs(data.buildArgs, null),
     ports: data.ports ?? [],
     dependsOn: data.dependsOn ?? [],
     environment: data.environment ?? {},
@@ -752,6 +754,9 @@ export async function updateService(
       patch.environment,
     );
   }
+  if ("buildArgs" in patch) {
+    patch.buildArgs = unmaskBuildArgs(patch.buildArgs, svc.buildArgs);
+  }
 
   // `advanced` is ONE blob holding independent, separately-owned keys —
   // `healthcheck` (edited in the service form), `readiness` (the deploy gate),
@@ -784,7 +789,11 @@ export async function updateService(
     // and be expanded on the next deploy.
     patch.advanced = mergeAdvanced(
       ("advanced" in patch ? patch.advanced : svc.advanced) as ComposeAdvanced | null,
-      { buildArgTemplateKeys: [] },
+      {
+        buildArgTemplateKeys: (
+          (svc.advanced as ComposeAdvanced | null)?.buildArgTemplateKeys ?? []
+        ).filter((key) => isMaskedValue(data.buildArgs?.[key])),
+      },
     );
   }
 
@@ -1368,6 +1377,7 @@ export async function syncComposeServices(
   const storedEnvByName = new Map(
     stored.map((s) => [s.name, (s.environment as Record<string, string> | null) ?? {}]),
   );
+  const storedArgsByName = new Map(stored.map((s) => [s.name, s.buildArgs]));
 
   // Import path, but the hostnames are still client-authored — same gate as the
   // create/update editors (normalizeRoutingPatch); `syncFromCompose` writes the
@@ -1410,6 +1420,9 @@ export async function syncComposeServices(
 
     return {
       ...svc,
+      ...(svc.buildArgs && {
+        buildArgs: unmaskBuildArgs(svc.buildArgs, storedArgsByName.get(svc.name)),
+      }),
       ...(environment && { environment }),
       ...(persistTemplateProvenance && { environmentTemplates }),
     };
@@ -1484,7 +1497,7 @@ export async function syncComposeServices(
     }
   }
 
-  return synced.map(maskServiceEnv);
+  return synced.map((svc) => maskServiceEnv(svc));
 }
 
 // ─── Service Deployments (per-deployment state) ──────────────────────────────