diff --git a/docs/env.md b/docs/env.md index a9e1eed56..339205d00 100644 --- a/docs/env.md +++ b/docs/env.md @@ -195,6 +195,16 @@ That means `.hack/.env` is no longer the primary runtime source of truth. `hack env materialize` is manual by design. Use it only when you need a compatibility file for an external tool that expects `.env` on disk. +Hack writes `.hack/.env` and `.hack/.env.` (from `materialize`, legacy +`hack env set`, and the daemon env route) with mode `0600` by atomic replace: a +private temporary file in `.hack/` is written, fsynced and renamed over the target, +so readers never see a partial file. An existing file with group or other access is +tightened to `0600` even when its contents are unchanged. These write and update paths +refuse, with a fixed message that names no file or value, an env file that is a symlink +(including a dangling one) or not a regular file, or one whose `.hack` directory is a +symlink. Replace the link with a regular file or directory to continue. Code paths +that only read an env file do not apply this check and still follow a symlink. + For host execution, `hack host exec`, `hack host shell`, and lifecycle hooks/processes use a host-local view. That means Hack prefers host-usable values when it can: @@ -529,7 +539,14 @@ The modern YAML env model is the canonical repo format today. Some older surfaces are still compatibility-oriented: -- `hack env backend ...` remains relevant for legacy repos and older secret-store configurations +- `hack env backend ...` remains relevant for legacy repos and older secret-store configurations. + The `encrypted_file` store (also used by the `cloud` shim) is replaced atomically + with mode `0600`, and set/delete hold an in-process queue plus a cooperative + `.lock` so concurrent writers, including separate processes, do not lose + updates. A store path that is a symlink is refused for writes. The v1 store + envelope and legacy `v1:` values authenticate no associated data: they are not + bound to a path, project or variable name beyond the scoped key inside the + encrypted store plaintext - the daemon's env HTTP route (used by the macOS app) reads and writes the legacy dotenv-shaped contract (`source: 'plain_env'|'keychain'`, `resolvedFrom: 'dotenv'/'process'/'keychain'/'portable_backend'`), not the modern YAML `hack env list --json` shape; there is no web dashboard surface anymore diff --git a/src/lib/hack-env.ts b/src/lib/hack-env.ts index 6a0277aeb..77ba543cf 100644 --- a/src/lib/hack-env.ts +++ b/src/lib/hack-env.ts @@ -6,8 +6,12 @@ import { } from "../constants.ts"; import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; import { parseDotEnv, serializeDotEnv } from "./env.ts"; -import { readTextFile, writeTextFileIfChanged } from "./fs.ts"; +import { readTextFile } from "./fs.ts"; import { getString, isRecord } from "./guards.ts"; +import { + readPrivateEnvFile, + writePrivateEnvFileIfChanged, +} from "./private-env-file.ts"; import { readProjectDefaultEnvConfig } from "./project.ts"; import type { SecretStoreDescriptor } from "./secret-store.ts"; import { @@ -502,19 +506,23 @@ export async function upsertDotEnvValue(opts: { readonly key: string; readonly value: string; }): Promise<{ readonly changed: boolean }> { - const existingText = (await readTextFile(opts.envFile)) ?? ""; + const existingText = + (await readPrivateEnvFile({ envFile: opts.envFile })) ?? ""; const env = parseDotEnv(existingText); const nextEnv: Record = { ...env, [opts.key]: opts.value }; const nextText = serializeDotEnvStable(nextEnv); - const result = await writeTextFileIfChanged(opts.envFile, nextText); - return { changed: result.changed }; + return await writePrivateEnvFileIfChanged({ + envFile: opts.envFile, + text: nextText, + }); } export async function removeDotEnvKey(opts: { readonly envFile: string; readonly key: string; }): Promise<{ readonly changed: boolean }> { - const existingText = (await readTextFile(opts.envFile)) ?? ""; + const existingText = + (await readPrivateEnvFile({ envFile: opts.envFile })) ?? ""; const env = parseDotEnv(existingText); if (!(opts.key in env)) { return { changed: false }; @@ -527,8 +535,10 @@ export async function removeDotEnvKey(opts: { nextEnv[key] = value; } const nextText = serializeDotEnvStable(nextEnv); - const result = await writeTextFileIfChanged(opts.envFile, nextText); - return { changed: result.changed }; + return await writePrivateEnvFileIfChanged({ + envFile: opts.envFile, + text: nextText, + }); } export function resolveKeychainServiceName(opts: { diff --git a/src/lib/private-env-file.ts b/src/lib/private-env-file.ts new file mode 100644 index 000000000..48e58a46c --- /dev/null +++ b/src/lib/private-env-file.ts @@ -0,0 +1,74 @@ +import { lstat, mkdir } from "node:fs/promises"; +import { dirname } from "node:path"; +import { isRecord } from "./guards.ts"; +import { + type ReplaceablePrivateFileOptions, + readReplaceablePrivateFile, + replacePrivateFileAtomic, +} from "./private-file.ts"; + +/** + * Plaintext env files under `.hack/` (`.hack/.env` and `.hack/.env.`). + * + * Ownership: Hack owns these files. They are read without following a symlink + * and replaced atomically with mode 0600. A symlink or non-regular entry at the + * file, or a symlinked `.hack` directory, is refused with + * {@link PRIVATE_ENV_FILE_REFUSAL}; the refusal never names the file, overlay + * or any value. Ancestors above `.hack` are not inspected. + */ +export const PRIVATE_ENV_FILE_REFUSAL = + "Refused .hack env file access: the env file or its .hack directory is a symlink or not a regular file. Replace it with a regular file or directory and retry; values omitted."; + +function privateEnvFileRefusal(): Error { + return new Error(PRIVATE_ENV_FILE_REFUSAL); +} + +function envFileOptions(envFile: string): ReplaceablePrivateFileOptions { + return { + path: envFile, + directories: [dirname(envFile)], + refusal: privateEnvFileRefusal, + }; +} + +/** Current text, or null when the env file is absent. */ +export async function readPrivateEnvFile(opts: { + readonly envFile: string; +}): Promise { + const current = await readReplaceablePrivateFile( + envFileOptions(opts.envFile) + ); + return current?.text ?? null; +} + +/** + * Writes `text` when it differs from the current contents, or when the existing + * file grants any group/other permission. `changed` reports a content change. + */ +export async function writePrivateEnvFileIfChanged(opts: { + readonly envFile: string; + readonly text: string; +}): Promise<{ readonly changed: boolean }> { + await ensureEnvFileDirectory({ directory: dirname(opts.envFile) }); + const options = envFileOptions(opts.envFile); + const existing = await readReplaceablePrivateFile(options); + const changed = existing?.text !== opts.text; + if (changed || ((existing?.mode ?? 0) & 0o077) !== 0) { + await replacePrivateFileAtomic({ ...options, text: opts.text }); + } + return { changed }; +} + +/** Creates a missing `.hack` directory; an existing entry is left to the refusal checks. */ +async function ensureEnvFileDirectory(opts: { + readonly directory: string; +}): Promise { + try { + await lstat(opts.directory); + } catch (error: unknown) { + if (!(isRecord(error) && error.code === "ENOENT")) { + throw error; + } + await mkdir(opts.directory, { recursive: true }); + } +} diff --git a/src/lib/private-file.ts b/src/lib/private-file.ts index fd7f02d6e..3776ef235 100644 --- a/src/lib/private-file.ts +++ b/src/lib/private-file.ts @@ -1,12 +1,14 @@ +import { randomUUID } from "node:crypto"; import { constants, type Stats } from "node:fs"; import { type FileHandle, lstat, open, realpath, + rename, unlink, } from "node:fs/promises"; -import { dirname } from "node:path"; +import { basename, dirname, join } from "node:path"; import { isRecord } from "./guards.ts"; /** @@ -199,6 +201,168 @@ export async function createPrivateFileExclusive(opts: { await syncDirectory({ path: dirname(opts.path) }); } +/** Options shared by the no-follow read and atomic replace of a private file. */ +export type ReplaceablePrivateFileOptions = { + readonly path: string; + /** + * Directories (normally the file's parent inside the project) that must be + * real directories rather than symlinks, with a stable identity across the + * operation. Ancestors outside this list are not inspected. + */ + readonly directories: readonly string[]; + /** Builds the caller's fixed, value-free refusal. */ + readonly refusal: () => Error; +}; + +/** Current contents and permission bits of a replaceable private file. */ +export type ReplaceablePrivateFile = { + readonly text: string; + readonly mode: number; +}; + +type DirectoryIdentity = { readonly path: string; readonly stats: Stats }; + +async function captureRealDirectories( + opts: ReplaceablePrivateFileOptions +): Promise { + return await Promise.all( + opts.directories.map(async (path) => { + const stats = await lstat(path).catch(() => null); + if (!stats?.isDirectory()) { + throw opts.refusal(); + } + return { path, stats }; + }) + ); +} + +async function recheckRealDirectories(opts: { + readonly directories: readonly DirectoryIdentity[]; + readonly refusal: () => Error; +}): Promise { + for (const { path, stats } of opts.directories) { + const current = await lstat(path).catch(() => null); + if ( + !current?.isDirectory() || + current.dev !== stats.dev || + current.ino !== stats.ino + ) { + throw opts.refusal(); + } + } +} + +/** Null when absent; refuses a symlink (dangling or not) or non-regular entry. */ +async function observeReplaceableTarget(opts: { + readonly path: string; + readonly refusal: () => Error; +}): Promise { + const observed = await lstat(opts.path).catch((error: unknown) => { + if (isMissingEntryError(error)) { + return null; + } + throw error; + }); + if (observed && !observed.isFile()) { + throw opts.refusal(); + } + return observed; +} + +/** + * Reads a private file that is later replaced with + * {@link replacePrivateFileAtomic}. Returns null when the entry is absent. A + * symlink at the path, a non-regular entry, a symlinked or replaced directory + * from `directories`, or a file that changes identity during the read throws + * `refusal()`; other filesystem errors (for example EACCES) propagate, so an + * unreadable file is never mistaken for an empty one and overwritten. + */ +export async function readReplaceablePrivateFile( + opts: ReplaceablePrivateFileOptions +): Promise { + const directories = await captureRealDirectories(opts); + const observed = await observeReplaceableTarget(opts); + if (!observed) { + await recheckRealDirectories({ directories, refusal: opts.refusal }); + return null; + } + let file: FileHandle; + try { + file = await open( + opts.path, + constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK + ); + } catch (error: unknown) { + if (isRecord(error) && error.code === "ELOOP") { + throw opts.refusal(); + } + throw error; + } + try { + const opened = await file.stat(); + if ( + !opened.isFile() || + opened.dev !== observed.dev || + opened.ino !== observed.ino + ) { + throw opts.refusal(); + } + const text = await file.readFile({ encoding: "utf8" }); + await recheckRealDirectories({ directories, refusal: opts.refusal }); + return { text, mode: opened.mode & 0o7777 }; + } finally { + await file.close(); + } +} + +/** + * Replaces `path` with `text` atomically and durably, leaving mode 0600. + * + * A uniquely named sibling is created with O_CREAT|O_EXCL|O_NOFOLLOW and mode + * 0600, fchmod 0600, written and fsynced, then renamed over `path`; readers see + * either the previous or the complete new contents, never a partial file. A + * symlink (dangling or not) or non-regular entry at `path`, or a symlinked or + * replaced directory from `directories`, throws `refusal()` before any write + * reaches the target. rename(2) never follows a final-component symlink, so + * even a link planted after the check is replaced rather than written through. + * The staging file is removed on every failure path. + */ +export async function replacePrivateFileAtomic( + opts: ReplaceablePrivateFileOptions & { readonly text: string } +): Promise { + const directories = await captureRealDirectories(opts); + await observeReplaceableTarget(opts); + const staging = join( + dirname(opts.path), + `.${basename(opts.path)}.${randomUUID()}.tmp` + ); + const file = await open( + staging, + constants.O_CREAT | + constants.O_EXCL | + constants.O_NOFOLLOW | + constants.O_WRONLY, + 0o600 + ); + let committed = false; + try { + // The umask can only remove bits; restore owner read/write explicitly. + await file.chmod(0o600); + await file.writeFile(opts.text, "utf8"); + await file.sync(); + await recheckRealDirectories({ directories, refusal: opts.refusal }); + await observeReplaceableTarget(opts); + await rename(staging, opts.path); + committed = true; + } finally { + await file.close(); + if (!committed) { + await unlink(staging).catch(() => undefined); + } + } + await syncDirectory({ path: dirname(opts.path) }); +} + /** Best effort: persist the directory entry where the platform allows it. */ async function syncDirectory(opts: { readonly path: string }): Promise { let directory: FileHandle | null = null; diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index 1d756962a..b4d0166e1 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -54,6 +54,7 @@ import { acquireNativeManagedEnvFile, assertNativeProjectInputRoot, } from "./native-project-inputs.ts"; +import { writePrivateEnvFileIfChanged } from "./private-env-file.ts"; import { createPrivateFileExclusive, decodeCanonicalBase64, @@ -2398,7 +2399,10 @@ export async function materializeProjectEnv(opts: { const envPath = resolve(opts.projectDir, PROJECT_ENV_FILENAME); const text = serializeDotEnv(selectedEnv); await assertLegacyProjectDirectory({ projectDir: opts.projectDir }); - const changed = (await writeTextFileIfChanged(envPath, text)).changed; + // Mode 0600, atomic replace; a symlinked .env or .hack is refused. + const changed = ( + await writePrivateEnvFileIfChanged({ envFile: envPath, text }) + ).changed; await ensureDir( dirname(resolveProjectEnvStatePath({ projectDir: opts.projectDir })) ); diff --git a/src/lib/projects-registry-lock.ts b/src/lib/projects-registry-lock.ts index 84f03bb4c..4448879fe 100644 --- a/src/lib/projects-registry-lock.ts +++ b/src/lib/projects-registry-lock.ts @@ -39,6 +39,8 @@ export async function withProjectsRegistryLock(opts: { readonly waitForLock?: boolean; readonly signal?: AbortSignal; readonly timeoutMs?: number; + /** Fixed resource name for the timeout message; defaults to the registry. */ + readonly label?: string; }): Promise { opts.signal?.throwIfAborted(); if (opts.waitForLock === false) { @@ -102,6 +104,7 @@ async function acquire(opts: { readonly waitForLock?: boolean; readonly signal?: AbortSignal; readonly timeoutMs?: number; + readonly label?: string; }): Promise { const started = performance.now(); const timeout = opts.timeoutMs ?? WAIT_MS; @@ -121,7 +124,7 @@ async function acquire(opts: { ? "; recovery guard is busy (interrupted recovery requires offline inspection)" : ""; throw new Error( - `Timed out waiting for projects registry lock${recovery}` + `Timed out waiting for ${opts.label ?? "projects registry"} lock${recovery}` ); } // Jitter prevents a burst of CLI clients from retrying in lockstep. diff --git a/src/lib/secret-store.ts b/src/lib/secret-store.ts index 847492244..5d56cdbef 100644 --- a/src/lib/secret-store.ts +++ b/src/lib/secret-store.ts @@ -15,7 +15,11 @@ import type { import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; import { resolveGlobalHackDir } from "./config-paths.ts"; import { ensureDir, readTextFile } from "./fs.ts"; -import { decodeCanonicalBase64 } from "./private-file.ts"; +import { + decodeCanonicalBase64, + replacePrivateFileAtomic, +} from "./private-file.ts"; +import { withProjectsRegistryLock } from "./projects-registry-lock.ts"; const ENCRYPTED_FILE_KEY_ENV = "HACK_SECRETS_FILE_KEY"; const DISABLE_ENCRYPTED_FILE_KEYCHAIN_FALLBACK_ENV = @@ -31,7 +35,11 @@ const ENCRYPTED_IV_BYTES = 12; const ENCRYPTED_TAG_BYTES = 16; /** Decoded ciphertext bound for the whole encrypted store. */ const ENCRYPTED_CIPHERTEXT_LIMIT = 16 * 1024 * 1024; +const ENCRYPTED_STORE_WRITE_REFUSAL = + "Refused encrypted secret store write: the store path is a symlink or not a regular file. Replace it with a regular file and retry; values omitted."; let cachedEncryptedFileKey: string | null = null; +/** Per-path tail of the in-process mutation queue; see withEncryptedStoreMutation. */ +const encryptedStoreMutationTails = new Map>(); let encryptedFileKeyFailureCooldownUntilMs = 0; type EncryptedStorePayload = { @@ -388,45 +396,91 @@ function createEncryptedScopedStore(input: { }); return entries[scopedKey] ?? null; }, - set: async ({ key, value }) => { - const entries = await readEncryptedEntries({ - filePath: input.filePath, - encryptionKey: input.encryptionKey, - }); - const scopedKey = buildScopedSecretKey({ - scope: input.scope, - key, - }); - entries[scopedKey] = value; - await writeEncryptedEntries({ - filePath: input.filePath, - encryptionKey: input.encryptionKey, - entries, - }); - }, - delete: async ({ key }) => { - const entries = await readEncryptedEntries({ + set: async ({ key, value }) => + await withEncryptedStoreMutation({ filePath: input.filePath, - encryptionKey: input.encryptionKey, - }); - const scopedKey = buildScopedSecretKey({ - scope: input.scope, - key, - }); - if (!(scopedKey in entries)) { - return false; - } - delete entries[scopedKey]; - await writeEncryptedEntries({ + run: async () => { + const entries = await readEncryptedEntries({ + filePath: input.filePath, + encryptionKey: input.encryptionKey, + }); + const scopedKey = buildScopedSecretKey({ + scope: input.scope, + key, + }); + entries[scopedKey] = value; + await writeEncryptedEntries({ + filePath: input.filePath, + encryptionKey: input.encryptionKey, + entries, + }); + }, + }), + delete: async ({ key }) => + await withEncryptedStoreMutation({ filePath: input.filePath, - encryptionKey: input.encryptionKey, - entries, - }); - return true; - }, + run: async () => { + const entries = await readEncryptedEntries({ + filePath: input.filePath, + encryptionKey: input.encryptionKey, + }); + const scopedKey = buildScopedSecretKey({ + scope: input.scope, + key, + }); + if (!(scopedKey in entries)) { + return false; + } + delete entries[scopedKey]; + await writeEncryptedEntries({ + filePath: input.filePath, + encryptionKey: input.encryptionKey, + entries, + }); + return true; + }, + }), }; } +/** + * Serializes read-modify-write of one encrypted store file. + * + * Writers in this process queue per configured path, so they never poll each + * other; across processes the cooperative `.lock` (the same exclusive + * hard-link owner protocol as the projects registry, with dead-owner recovery) + * admits one writer at a time. Every set/delete re-reads the store inside the + * lock, so a concurrent writer's update is never lost. Reads take no lock: + * writes replace the file atomically, so a reader sees a whole envelope. + */ +async function withEncryptedStoreMutation(input: { + readonly filePath: string; + readonly run: () => Promise; +}): Promise { + const previous = + encryptedStoreMutationTails.get(input.filePath) ?? Promise.resolve(); + let release = (): void => undefined; + const turn = new Promise((resolveTurn) => { + release = resolveTurn; + }); + const tail = previous.then(() => turn); + encryptedStoreMutationTails.set(input.filePath, tail); + await previous; + try { + await ensureDir(dirname(input.filePath)); + return await withProjectsRegistryLock({ + lockPath: `${input.filePath}.lock`, + label: "encrypted secret store", + run: input.run, + }); + } finally { + release(); + if (encryptedStoreMutationTails.get(input.filePath) === tail) { + encryptedStoreMutationTails.delete(input.filePath); + } + } +} + function buildScopedSecretKey(input: { readonly scope: string; readonly key: string; @@ -751,7 +805,14 @@ async function writeEncryptedEntries(input: { encryptionKey: input.encryptionKey, }); await ensureDir(dirname(input.filePath)); - await Bun.write(input.filePath, `${JSON.stringify(envelope, null, 2)}\n`); + // Atomic same-directory replace with mode 0600. The configured store + // location's ancestors are the user's choice and are not inspected. + await replacePrivateFileAtomic({ + path: input.filePath, + directories: [], + text: `${JSON.stringify(envelope, null, 2)}\n`, + refusal: () => new Error(ENCRYPTED_STORE_WRITE_REFUSAL), + }); } /** diff --git a/tests/aes-gcm-v1-context-binding.test.ts b/tests/aes-gcm-v1-context-binding.test.ts new file mode 100644 index 000000000..3f29993bb --- /dev/null +++ b/tests/aes-gcm-v1-context-binding.test.ts @@ -0,0 +1,482 @@ +/** + * Characterization of v1 AES-GCM context binding (HACK-1127). + * + * These tests pin existing behavior; they do not change or endorse it. Both v1 + * formats (the encrypted_file store envelope and the legacy `v1:` secure env + * value) derive the AES-256 key as SHA-256(key text), use a 12-byte IV and a + * 16-byte tag, and authenticate NO associated data. Consequently a v1 envelope + * is not bound to its file path, project, scope, overlay or variable name: + * + * - the store binds an entry to its project only through the scoped key inside + * the authenticated plaintext (`encodeURIComponent(scope)::encodeURIComponent(key)`); + * - a legacy `v1:` value moved to another variable or scope still decrypts. + * + * Adding context binding requires a new envelope version; a v1 change that made + * any of these tests fail would break existing v1 stores and values. + */ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import * as nodeCrypto from "node:crypto"; +import { + copyFile, + mkdir, + mkdtemp, + readFile, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { YAML } from "bun"; + +import { + PROJECT_COMPOSE_FILENAME, + PROJECT_CONFIG_FILENAME, + PROJECT_ENV_KEY_FILENAME, +} from "../src/constants.ts"; +import { + resolveProjectEnvConfig, + setProjectEnvValue, +} from "../src/lib/project-env-config.ts"; +import { resolveSecretStore } from "../src/lib/secret-store.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +const STORE_KEY = "synthetic-v1-binding-store-key"; +const ENV_KEY = "synthetic-v1-binding-env-key"; +const SENTINEL = "synthetic-v1-binding-value-never-in-errors"; +const OTHER_SENTINEL = "synthetic-v1-binding-other-value"; +/** Fixed test vector IV; production always draws a random IV. */ +const FIXED_IV = Buffer.from("000102030405060708090a0b", "hex"); +const ISOLATED_ENV_KEYS = [ + "HOME", + "HACK_HOME", + "HACK_GLOBAL_CONFIG_PATH", + "HACK_SECRETS_FILE_KEY", + "HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", + "HACK_ENV_SECRET_KEY", +] as const; + +let root: string | null = null; +const previousEnv = new Map(); + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "hack-v1-binding-")); + for (const key of ISOLATED_ENV_KEYS) { + previousEnv.set(key, process.env[key]); + } + process.env.HOME = join(root, "home"); + process.env.HACK_HOME = join(root, "home", ".hack"); + process.env.HACK_GLOBAL_CONFIG_PATH = join(root, "home", "hack.config.json"); + process.env.HACK_SECRETS_FILE_KEY = STORE_KEY; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + Reflect.deleteProperty(process.env, "HACK_ENV_SECRET_KEY"); + await mkdir(join(root, "home"), { recursive: true }); +}); + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }); + root = null; + } + for (const key of ISOLATED_ENV_KEYS) { + restoreEnv(key, previousEnv.get(key)); + } + previousEnv.clear(); +}); + +function requireRoot(): string { + if (!root) { + throw new Error("Missing temp root"); + } + return root; +} + +function deriveKey(keyText: string): Buffer { + return nodeCrypto.createHash("sha256").update(keyText, "utf8").digest(); +} + +/** AES-256-GCM seal with an optional AAD, mirroring the v1 parameters. */ +function seal(opts: { + readonly keyText: string; + readonly plaintext: string; + readonly aad?: Buffer; +}): { readonly iv: Buffer; readonly tag: Buffer; readonly ciphertext: Buffer } { + const cipher = nodeCrypto.createCipheriv( + "aes-256-gcm", + deriveKey(opts.keyText), + FIXED_IV, + { authTagLength: 16 } + ); + if (opts.aad) { + cipher.setAAD(opts.aad); + } + const ciphertext = Buffer.concat([ + cipher.update(opts.plaintext, "utf8"), + cipher.final(), + ]); + return { iv: FIXED_IV, tag: cipher.getAuthTag(), ciphertext }; +} + +function open(opts: { + readonly keyText: string; + readonly iv: string; + readonly tag: string; + readonly ciphertext: string; +}): string { + const decipher = nodeCrypto.createDecipheriv( + "aes-256-gcm", + deriveKey(opts.keyText), + Buffer.from(opts.iv, "base64"), + { authTagLength: 16 } + ); + decipher.setAuthTag(Buffer.from(opts.tag, "base64")); + return Buffer.concat([ + decipher.update(Buffer.from(opts.ciphertext, "base64")), + decipher.final(), + ]).toString("utf8"); +} + +// --------------------------------------------------------------------------- +// encrypted_file store envelope +// --------------------------------------------------------------------------- + +async function configureStore(opts: { + readonly storePath: string; + readonly cloud?: boolean; +}): Promise { + const configPath = process.env.HACK_GLOBAL_CONFIG_PATH; + if (!configPath) { + throw new Error("Missing HACK_GLOBAL_CONFIG_PATH"); + } + const secrets = opts.cloud + ? { + backend: "cloud", + encryptedFile: { path: opts.storePath }, + cloud: { + provider: "aws", + project: "dev-account", + secretPrefix: "hack-cli", + }, + } + : { backend: "encrypted_file", encryptedFile: { path: opts.storePath } }; + await writeFile( + configPath, + `${JSON.stringify({ controlPlane: { secrets } })}\n` + ); +} + +function storeEnvelopeText(opts: { + readonly plaintext: string; + readonly aad?: Buffer; +}): string { + const sealed = seal({ keyText: STORE_KEY, ...opts }); + return `${JSON.stringify({ + version: 1, + algorithm: "aes-256-gcm", + iv: sealed.iv.toString("base64"), + tag: sealed.tag.toString("base64"), + ciphertext: sealed.ciphertext.toString("base64"), + })}\n`; +} + +const STORE_PLAINTEXT = JSON.stringify({ + version: 1, + entries: { "app::TOKEN": SENTINEL }, +}); + +test("v1 store: written envelope has the pinned fields and a scoped-key plaintext", async () => { + const storePath = resolve(requireRoot(), "secrets.enc.json"); + await configureStore({ storePath }); + const store = await resolveSecretStore({ projectName: "app" }); + await store.set({ key: "TOKEN", value: SENTINEL }); + + const envelope = JSON.parse(await readFile(storePath, "utf8")) as Record< + string, + string | number + >; + expect(Object.keys(envelope)).toEqual([ + "version", + "algorithm", + "iv", + "tag", + "ciphertext", + ]); + expect(envelope.version).toBe(1); + expect(envelope.algorithm).toBe("aes-256-gcm"); + // Opening with SHA-256(key), no AAD, reproduces the exact v1 plaintext. + expect( + open({ + keyText: STORE_KEY, + iv: String(envelope.iv), + tag: String(envelope.tag), + ciphertext: String(envelope.ciphertext), + }) + ).toBe(STORE_PLAINTEXT); +}); + +test("v1 store: a test-vector envelope sealed without AAD opens", async () => { + const storePath = resolve(requireRoot(), "secrets.enc.json"); + await configureStore({ storePath }); + await writeFile(storePath, storeEnvelopeText({ plaintext: STORE_PLAINTEXT })); + const store = await resolveSecretStore({ projectName: "app" }); + expect(await store.get({ key: "TOKEN" })).toBe(SENTINEL); +}); + +for (const [label, aad] of [ + ["the store path", (storePath: string) => Buffer.from(storePath, "utf8")], + ["the project scope", () => Buffer.from("app", "utf8")], + ["the envelope version", () => Buffer.from("v1", "utf8")], +] as const) { + test(`v1 store: an envelope sealed with ${label} as AAD is refused (no AAD is authenticated)`, async () => { + const storePath = resolve(requireRoot(), "secrets.enc.json"); + await configureStore({ storePath }); + await writeFile( + storePath, + storeEnvelopeText({ plaintext: STORE_PLAINTEXT, aad: aad(storePath) }) + ); + const store = await resolveSecretStore({ projectName: "app" }); + const error = await store.get({ key: "TOKEN" }).then( + () => null, + (caught: unknown) => caught + ); + expect(error).toBeInstanceOf(Error); + expect(String(error)).toContain( + "Cannot decrypt encrypted secret store at " + ); + for (const privateText of [SENTINEL, STORE_KEY]) { + expect(String(error)).not.toContain(privateText); + } + }); +} + +test("v1 store: an envelope is not bound to its file path", async () => { + const firstPath = resolve(requireRoot(), "first.enc.json"); + await configureStore({ storePath: firstPath }); + const first = await resolveSecretStore({ projectName: "app" }); + await first.set({ key: "TOKEN", value: SENTINEL }); + + const movedPath = resolve(requireRoot(), "moved", "second.enc.json"); + await mkdir(resolve(requireRoot(), "moved")); + await copyFile(firstPath, movedPath); + await configureStore({ storePath: movedPath }); + const moved = await resolveSecretStore({ projectName: "app" }); + expect(moved.descriptor.location).toBe(movedPath); + expect(await moved.get({ key: "TOKEN" })).toBe(SENTINEL); +}); + +test("v1 store: project binding lives only in the scoped plaintext key", async () => { + const storePath = resolve(requireRoot(), "secrets.enc.json"); + await configureStore({ storePath }); + // A test-vector entry under another project's scoped key is visible to that + // project and invisible to this one; nothing outside the plaintext binds it. + await writeFile( + storePath, + storeEnvelopeText({ + plaintext: JSON.stringify({ + version: 1, + entries: { + "app::TOKEN": SENTINEL, + "my%20app::TOKEN": OTHER_SENTINEL, + }, + }), + }) + ); + expect( + await (await resolveSecretStore({ projectName: "app" })).get({ + key: "TOKEN", + }) + ).toBe(SENTINEL); + expect( + await (await resolveSecretStore({ projectName: "other" })).get({ + key: "TOKEN", + }) + ).toBeNull(); + expect( + await (await resolveSecretStore({ projectName: "my app" })).get({ + key: "TOKEN", + }) + ).toBe(OTHER_SENTINEL); +}); + +test("v1 store: the cloud shim scope is a plaintext key prefix in the same envelope", async () => { + const storePath = resolve(requireRoot(), "cloud.enc.json"); + await configureStore({ storePath, cloud: true }); + const store = await resolveSecretStore({ projectName: "app" }); + await store.set({ key: "API_TOKEN", value: SENTINEL }); + const envelope = JSON.parse(await readFile(storePath, "utf8")) as Record< + string, + string + >; + const plaintext = JSON.parse( + open({ + keyText: STORE_KEY, + iv: String(envelope.iv), + tag: String(envelope.tag), + ciphertext: String(envelope.ciphertext), + }) + ) as { readonly entries: Record }; + expect(plaintext.entries).toEqual({ + "cloud%3Aaws%3Adev-account%3Ahack-cli%3Aapp::API_TOKEN": SENTINEL, + }); +}); + +// --------------------------------------------------------------------------- +// legacy `v1:` secure env values +// --------------------------------------------------------------------------- + +async function createLegacyProject(): Promise<{ + readonly projectRoot: string; + readonly projectDir: string; + readonly configPath: string; +}> { + const projectRoot = resolve(requireRoot(), "repo"); + const projectDir = resolve(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, PROJECT_COMPOSE_FILENAME), + "services:\n api: {}\n" + ); + await writeFile( + resolve(projectDir, PROJECT_CONFIG_FILENAME), + `${JSON.stringify({ name: "v1-binding", dev_host: "v1-binding.hack" })}\n` + ); + await writeFile( + resolve(projectRoot, PROJECT_ENV_KEY_FILENAME), + `${ENV_KEY}\n` + ); + return { + projectRoot, + projectDir, + configPath: resolve(projectDir, "hack.env.default.yaml"), + }; +} + +type LegacyConfig = { + readonly version: number; + readonly environment: string; + readonly secretsprovider: string; + readonly values: Record>; +}; + +async function readLegacyConfig(path: string): Promise { + return YAML.parse(await readFile(path, "utf8")) as LegacyConfig; +} + +function secureOf(config: LegacyConfig, scope: string, key: string): string { + const secure = config.values[scope]?.[key]?.secure; + if (typeof secure !== "string") { + throw new Error("missing secure value"); + } + return secure; +} + +async function resolveLegacy(project: { + readonly projectRoot: string; + readonly projectDir: string; +}) { + const resolved = await resolveProjectEnvConfig({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + serviceNames: ["api"], + }); + if (!resolved) { + throw new Error("missing resolved config"); + } + return resolved; +} + +test("legacy v1: a secure value swapped across variable and scope still decrypts", async () => { + const project = await createLegacyProject(); + for (const [scope, key, value] of [ + ["global", "TOKEN", SENTINEL], + ["api", "OTHER", OTHER_SENTINEL], + ] as const) { + await setProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope, + key, + value, + secret: true, + }); + } + const config = await readLegacyConfig(project.configPath); + const token = secureOf(config, "global", "TOKEN"); + const other = secureOf(config, "api", "OTHER"); + expect(token.split(":")[0]).toBe("v1"); + expect(Buffer.from(token.split(":")[1] ?? "", "base64").length).toBe(12); + + await writeFile( + project.configPath, + JSON.stringify({ + ...config, + values: { + global: { TOKEN: { secure: other } }, + api: { OTHER: { secure: token } }, + }, + }) + ); + const resolved = await resolveLegacy(project); + expect(resolved.globalEnv.TOKEN).toBe(OTHER_SENTINEL); + expect(resolved.serviceEnv.api?.OTHER).toBe(SENTINEL); +}); + +function legacyValue(aad?: Buffer): string { + const sealed = seal({ keyText: ENV_KEY, plaintext: SENTINEL, aad }); + return [ + "v1", + sealed.iv.toString("base64"), + sealed.tag.toString("base64"), + sealed.ciphertext.toString("base64"), + ].join(":"); +} + +async function writeLegacyValue(opts: { + readonly configPath: string; + readonly secure: string; +}): Promise { + await writeFile( + opts.configPath, + JSON.stringify({ + version: 1, + environment: "default", + secretsprovider: "project_key", + values: { global: { TOKEN: { secure: opts.secure } } }, + }) + ); +} + +test("legacy v1: a test-vector value sealed without AAD decrypts", async () => { + const project = await createLegacyProject(); + await writeLegacyValue({ + configPath: project.configPath, + secure: legacyValue(), + }); + expect((await resolveLegacy(project)).globalEnv.TOKEN).toBe(SENTINEL); +}); + +for (const [label, aad] of [ + ["the variable name", Buffer.from("TOKEN", "utf8")], + ["the scope", Buffer.from("global", "utf8")], + ["the version prefix", Buffer.from("v1", "utf8")], +] as const) { + test(`legacy v1: a value sealed with ${label} as AAD is refused (no AAD is authenticated)`, async () => { + const project = await createLegacyProject(); + await writeLegacyValue({ + configPath: project.configPath, + secure: legacyValue(aad), + }); + const error = await resolveLegacy(project).then( + () => null, + (caught: unknown) => caught + ); + expect(error).toBeInstanceOf(Error); + expect(error instanceof Error ? error.message : "").toBe( + "Invalid secure env value." + ); + for (const privateText of [SENTINEL, ENV_KEY]) { + expect(String(error)).not.toContain(privateText); + } + }); +} diff --git a/tests/private-env-file.test.ts b/tests/private-env-file.test.ts new file mode 100644 index 000000000..798792cae --- /dev/null +++ b/tests/private-env-file.test.ts @@ -0,0 +1,391 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + chmod, + link, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { + PROJECT_COMPOSE_FILENAME, + PROJECT_CONFIG_FILENAME, + PROJECT_ENV_FILENAME, +} from "../src/constants.ts"; +import { removeDotEnvKey, upsertDotEnvValue } from "../src/lib/hack-env.ts"; +import { PRIVATE_ENV_FILE_REFUSAL } from "../src/lib/private-env-file.ts"; +import { replacePrivateFileAtomic } from "../src/lib/private-file.ts"; +import { + materializeProjectEnv, + setProjectEnvValue, +} from "../src/lib/project-env-config.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +// Synthetic values; none of them may appear in a refusal. +const SENTINEL_KEY = "SYNTHETIC_PRIVATE_ENV_KEY"; +const SENTINEL_VALUE = "synthetic-private-env-value-never-in-errors"; +const OUTSIDE_TEXT = "OUTSIDE_SENTINEL=synthetic-outside-file-never-read\n"; + +let root: string | null = null; +let previousHome: string | undefined; +let previousGlobalConfigPath: string | undefined; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "hack-private-env-file-")); + previousHome = process.env.HOME; + previousGlobalConfigPath = process.env.HACK_GLOBAL_CONFIG_PATH; + // No real HOME or global config is read or written by these tests. + process.env.HOME = join(root, "home"); + process.env.HACK_GLOBAL_CONFIG_PATH = join(root, "home", "hack.config.json"); + await mkdir(join(root, "home"), { recursive: true }); +}); + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }); + root = null; + } + restoreEnv("HOME", previousHome); + restoreEnv("HACK_GLOBAL_CONFIG_PATH", previousGlobalConfigPath); +}); + +async function createProject(): Promise<{ + readonly projectRoot: string; + readonly projectDir: string; + readonly envFile: string; + readonly outsideFile: string; +}> { + if (!root) { + throw new Error("Missing temp root"); + } + const projectRoot = resolve(root, "repo"); + const projectDir = resolve(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, PROJECT_COMPOSE_FILENAME), + "services:\n api: {}\n" + ); + await writeFile( + resolve(projectDir, PROJECT_CONFIG_FILENAME), + `${JSON.stringify({ name: "private-env-test", dev_host: "private-env.hack" })}\n` + ); + const outsideFile = resolve(root, "outside.env"); + await writeFile(outsideFile, OUTSIDE_TEXT); + return { + projectRoot, + projectDir, + envFile: resolve(projectDir, PROJECT_ENV_FILENAME), + outsideFile, + }; +} + +function expectClosedRefusal(error: unknown, paths: readonly string[]): void { + expect(error).toBeInstanceOf(Error); + expect(error instanceof Error ? error.message : "").toBe( + PRIVATE_ENV_FILE_REFUSAL + ); + for (const privateText of [ + SENTINEL_KEY, + SENTINEL_VALUE, + "OUTSIDE_SENTINEL", + ...paths, + ]) { + expect(String(error)).not.toContain(privateText); + } +} + +async function caught(promise: Promise): Promise { + return await promise.then( + () => null, + (error: unknown) => error + ); +} + +async function stagingLeftovers(directory: string): Promise { + return (await readdir(directory)).filter((name) => name.endsWith(".tmp")); +} + +test("upsertDotEnvValue creates .hack/.env with mode 0600", async () => { + const project = await createProject(); + const result = await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }); + expect(result.changed).toBe(true); + expect((await stat(project.envFile)).mode & 0o777).toBe(0o600); + expect(await readFile(project.envFile, "utf8")).toContain( + `${SENTINEL_KEY}=${SENTINEL_VALUE}` + ); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); + +test("upsertDotEnvValue replaces .hack/.env atomically rather than writing in place", async () => { + const project = await createProject(); + await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: "first", + }); + // A second name for the old inode keeps the old bytes only if the writer + // renamed a new file over the path instead of truncating and rewriting it. + const previousName = resolve(project.projectDir, "previous-inode"); + await link(project.envFile, previousName); + const before = await stat(project.envFile); + + await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: "second", + }); + + const after = await stat(project.envFile); + expect(after.ino).not.toBe(before.ino); + expect(after.mode & 0o777).toBe(0o600); + expect(await readFile(previousName, "utf8")).toContain( + `${SENTINEL_KEY}=first` + ); + expect(await readFile(project.envFile, "utf8")).toContain( + `${SENTINEL_KEY}=second` + ); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); + +test("an unchanged .hack/.env with group or other bits is tightened to 0600", async () => { + const project = await createProject(); + await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }); + const text = await readFile(project.envFile, "utf8"); + await chmod(project.envFile, 0o644); + + const result = await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }); + expect(result.changed).toBe(false); + expect((await stat(project.envFile)).mode & 0o777).toBe(0o600); + expect(await readFile(project.envFile, "utf8")).toBe(text); +}); + +test("removeDotEnvKey keeps mode 0600 on the rewritten file", async () => { + const project = await createProject(); + await upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }); + await upsertDotEnvValue({ + envFile: project.envFile, + key: "KEEP", + value: "1", + }); + const result = await removeDotEnvKey({ + envFile: project.envFile, + key: SENTINEL_KEY, + }); + expect(result.changed).toBe(true); + expect((await stat(project.envFile)).mode & 0o777).toBe(0o600); + expect(await readFile(project.envFile, "utf8")).toBe("KEEP=1\n"); +}); + +test("negative control: a symlinked .hack/.env is refused without reading or writing its target", async () => { + const project = await createProject(); + await symlink(project.outsideFile, project.envFile); + + for (const attempt of [ + upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }), + removeDotEnvKey({ envFile: project.envFile, key: "OUTSIDE_SENTINEL" }), + ]) { + expectClosedRefusal(await caught(attempt), [ + project.envFile, + project.outsideFile, + ]); + } + + expect(await readFile(project.outsideFile, "utf8")).toBe(OUTSIDE_TEXT); + expect((await lstat(project.envFile)).isSymbolicLink()).toBe(true); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); + +test("negative control: a dangling .hack/.env symlink is refused and its target is never created", async () => { + const project = await createProject(); + const missingTarget = resolve(project.projectRoot, "..", "never-created.env"); + await symlink(missingTarget, project.envFile); + + expectClosedRefusal( + await caught( + upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }) + ), + [project.envFile, missingTarget] + ); + await expect(lstat(missingTarget)).rejects.toThrow(); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); + +test("negative control: an env overlay file that is a symlink is refused", async () => { + const project = await createProject(); + const overlayFile = `${project.envFile}.staging`; + await symlink(project.outsideFile, overlayFile); + + expectClosedRefusal( + await caught( + upsertDotEnvValue({ + envFile: overlayFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }) + ), + [overlayFile, project.outsideFile, "staging"] + ); + expect(await readFile(project.outsideFile, "utf8")).toBe(OUTSIDE_TEXT); +}); + +test("negative control: a symlinked .hack directory is refused and the linked directory is untouched", async () => { + if (!root) { + throw new Error("Missing temp root"); + } + const outsideDir = resolve(root, "outside-hack"); + await mkdir(outsideDir); + const projectRoot = resolve(root, "linked-repo"); + await mkdir(projectRoot); + const projectDir = resolve(projectRoot, ".hack"); + await symlink(outsideDir, projectDir); + const envFile = resolve(projectDir, PROJECT_ENV_FILENAME); + + expectClosedRefusal( + await caught( + upsertDotEnvValue({ envFile, key: SENTINEL_KEY, value: SENTINEL_VALUE }) + ), + [envFile, outsideDir] + ); + expect(await readdir(outsideDir)).toEqual([]); +}); + +test("a directory at the .hack/.env path is refused with the same closed diagnostic", async () => { + const project = await createProject(); + await mkdir(project.envFile); + expectClosedRefusal( + await caught( + upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }) + ), + [project.envFile] + ); +}); + +test("materializeProjectEnv writes .hack/.env with mode 0600 by atomic replace", async () => { + const project = await createProject(); + await setProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + secret: false, + }); + await writeFile(project.envFile, "STALE=1\n", { mode: 0o644 }); + await chmod(project.envFile, 0o644); + const previousName = resolve(project.projectDir, "previous-inode"); + await link(project.envFile, previousName); + + const result = await materializeProjectEnv({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + serviceName: null, + serviceNames: ["api"], + }); + + expect(result.changed).toBe(true); + expect((await stat(project.envFile)).mode & 0o777).toBe(0o600); + expect(await readFile(project.envFile, "utf8")).toContain( + `${SENTINEL_KEY}=${SENTINEL_VALUE}` + ); + expect(await readFile(previousName, "utf8")).toBe("STALE=1\n"); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); + +test("negative control: materializeProjectEnv refuses a symlinked .hack/.env target", async () => { + const project = await createProject(); + await setProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + secret: false, + }); + await symlink(project.outsideFile, project.envFile); + + expectClosedRefusal( + await caught( + materializeProjectEnv({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + serviceName: null, + serviceNames: ["api"], + }) + ), + [project.envFile, project.outsideFile] + ); + expect(await readFile(project.outsideFile, "utf8")).toBe(OUTSIDE_TEXT); + expect((await lstat(project.envFile)).isSymbolicLink()).toBe(true); +}); + +test("negative control: replacePrivateFileAtomic itself refuses a symlinked target without the read guard", async () => { + const project = await createProject(); + const danglingTarget = resolve( + project.projectRoot, + "..", + "never-created.env" + ); + const danglingLink = `${project.envFile}.dangling`; + await symlink(project.outsideFile, project.envFile); + await symlink(danglingTarget, danglingLink); + + // Called directly, so the earlier O_NOFOLLOW read in the env-file callers + // cannot mask a missing check in the replace step. + for (const path of [project.envFile, danglingLink]) { + expectClosedRefusal( + await caught( + replacePrivateFileAtomic({ + path, + directories: [project.projectDir], + text: `${SENTINEL_KEY}=${SENTINEL_VALUE}\n`, + refusal: () => new Error(PRIVATE_ENV_FILE_REFUSAL), + }) + ), + [path, project.outsideFile, danglingTarget] + ); + expect((await lstat(path)).isSymbolicLink()).toBe(true); + } + expect(await readFile(project.outsideFile, "utf8")).toBe(OUTSIDE_TEXT); + await expect(lstat(danglingTarget)).rejects.toThrow(); + expect(await stagingLeftovers(project.projectDir)).toEqual([]); +}); diff --git a/tests/secret-store-file-hygiene.test.ts b/tests/secret-store-file-hygiene.test.ts new file mode 100644 index 000000000..3be32e5c7 --- /dev/null +++ b/tests/secret-store-file-hygiene.test.ts @@ -0,0 +1,248 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + link, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { resolveSecretStore } from "../src/lib/secret-store.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +const STORE_KEY = "synthetic-store-hygiene-key-for-tests-only"; +const STORE_SENTINEL = "synthetic-store-hygiene-value-never-in-errors"; +const SECRET_STORE_MODULE = resolve( + import.meta.dir, + "..", + "src", + "lib", + "secret-store.ts" +); +const WRITES_PER_PROCESS = 20; +const ISOLATED_ENV_KEYS = [ + "HOME", + "HACK_HOME", + "HACK_GLOBAL_CONFIG_PATH", + "HACK_SECRETS_FILE_KEY", + "HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", +] as const; + +let root: string | null = null; +const previousEnv = new Map(); + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "hack-secret-store-hygiene-")); + for (const key of ISOLATED_ENV_KEYS) { + previousEnv.set(key, process.env[key]); + } + // Temp HOME and global config; the env key wins, and the keychain fallback is + // disabled so no test can reach the native keychain. + process.env.HOME = join(root, "home"); + process.env.HACK_HOME = join(root, "home", ".hack"); + process.env.HACK_GLOBAL_CONFIG_PATH = join(root, "home", "hack.config.json"); + process.env.HACK_SECRETS_FILE_KEY = STORE_KEY; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + await mkdir(join(root, "home"), { recursive: true }); +}); + +afterEach(async () => { + if (root) { + await rm(root, { recursive: true, force: true }); + root = null; + } + for (const key of ISOLATED_ENV_KEYS) { + restoreEnv(key, previousEnv.get(key)); + } + previousEnv.clear(); +}); + +async function configureEncryptedFileStore(): Promise<{ + readonly storeDir: string; + readonly storePath: string; +}> { + if (!root) { + throw new Error("Missing temp root"); + } + const storeDir = resolve(root, "store"); + await mkdir(storeDir); + const storePath = resolve(storeDir, "secrets.enc.json"); + const configPath = process.env.HACK_GLOBAL_CONFIG_PATH; + if (!configPath) { + throw new Error("Missing HACK_GLOBAL_CONFIG_PATH"); + } + await writeFile( + configPath, + `${JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath }, + }, + }, + })}\n` + ); + return { storeDir, storePath }; +} + +async function nonEnvelopeEntries(directory: string): Promise { + return (await readdir(directory)).filter( + (name) => name !== "secrets.enc.json" + ); +} + +test("encrypted_file store writes mode 0600 by atomic replace", async () => { + const { storeDir, storePath } = await configureEncryptedFileStore(); + const store = await resolveSecretStore({ projectName: "app" }); + await store.set({ key: "TOKEN", value: "first" }); + expect((await stat(storePath)).mode & 0o777).toBe(0o600); + + // The old inode keeps its bytes only when the writer renames a new file over + // the path instead of truncating and rewriting the existing one. + const previousName = resolve(storeDir, "previous-inode"); + await link(storePath, previousName); + const previousText = await readFile(storePath, "utf8"); + const before = await stat(storePath); + + await store.set({ key: "TOKEN", value: STORE_SENTINEL }); + + const after = await stat(storePath); + expect(after.ino).not.toBe(before.ino); + expect(after.mode & 0o777).toBe(0o600); + expect(await readFile(previousName, "utf8")).toBe(previousText); + expect(await store.get({ key: "TOKEN" })).toBe(STORE_SENTINEL); + expect(await readFile(storePath, "utf8")).not.toContain(STORE_SENTINEL); + + expect(await store.delete({ key: "TOKEN" })).toBe(true); + expect((await stat(storePath)).mode & 0o777).toBe(0o600); + // No staging file or lock receipt is left behind. + expect(await nonEnvelopeEntries(storeDir)).toEqual(["previous-inode"]); +}); + +test("encrypted_file store refuses to write through a symlinked store path", async () => { + if (!root) { + throw new Error("Missing temp root"); + } + const { storeDir, storePath } = await configureEncryptedFileStore(); + const outsideFile = resolve(root, "outside-store.json"); + // An empty target reads as an empty store, so only the write guard can stop + // the set below. + await writeFile(outsideFile, ""); + await symlink(outsideFile, storePath); + + const store = await resolveSecretStore({ projectName: "app" }); + const error = await store.set({ key: "TOKEN", value: STORE_SENTINEL }).then( + () => null, + (caught: unknown) => caught + ); + + expect(error).toBeInstanceOf(Error); + expect(error instanceof Error ? error.message : "").toBe( + "Refused encrypted secret store write: the store path is a symlink or not a regular file. Replace it with a regular file and retry; values omitted." + ); + for (const privateText of [STORE_SENTINEL, STORE_KEY, "TOKEN", outsideFile]) { + expect(String(error)).not.toContain(privateText); + } + expect(await readFile(outsideFile, "utf8")).toBe(""); + expect((await lstat(storePath)).isSymbolicLink()).toBe(true); + expect(await nonEnvelopeEntries(storeDir)).toEqual([]); +}); + +test("negative control: concurrent in-process writers do not lose an update", async () => { + const { storeDir } = await configureEncryptedFileStore(); + const writers = await Promise.all( + ["app", "app", "other", "other"].map((projectName) => + resolveSecretStore({ projectName }) + ) + ); + const writes: Promise[] = []; + for (let index = 0; index < 24; index += 1) { + const writer = writers[index % writers.length]; + if (!writer) { + throw new Error("missing writer"); + } + writes.push(writer.set({ key: `KEY_${index}`, value: `value-${index}` })); + } + await Promise.all(writes); + + for (const projectName of ["app", "other"]) { + const store = await resolveSecretStore({ projectName }); + for (let index = 0; index < 24; index += 1) { + const writtenBy = index % 4 < 2 ? "app" : "other"; + expect(await store.get({ key: `KEY_${index}` })).toBe( + writtenBy === projectName ? `value-${index}` : null + ); + } + } + expect(await nonEnvelopeEntries(storeDir)).toEqual([]); +}); + +test("negative control: concurrent writer processes do not lose an update", async () => { + if (!root) { + throw new Error("Missing temp root"); + } + const { storeDir } = await configureEncryptedFileStore(); + const script = resolve(root, "store-writer.ts"); + await writeFile( + script, + [ + `import { resolveSecretStore } from ${JSON.stringify(SECRET_STORE_MODULE)};`, + "const prefix = process.argv[2] ?? 'P';", + "const startAt = Number(process.argv[3]);", + "const store = await resolveSecretStore({ projectName: 'app' });", + "// Start together so the writers' read-modify-write cycles overlap.", + "await Bun.sleep(Math.max(0, startAt - Date.now()));", + `for (let index = 0; index < ${WRITES_PER_PROCESS}; index += 1) {`, + " await store.set({ key: `${prefix}_${index}`, value: `${prefix}-${index}` });", + "}", + "", + ].join("\n") + ); + const childEnv: Record = { + PATH: process.env.PATH ?? "", + TMPDIR: process.env.TMPDIR ?? tmpdir(), + }; + for (const key of ISOLATED_ENV_KEYS) { + const value = process.env[key]; + if (value !== undefined) { + childEnv[key] = value; + } + } + const prefixes = ["A", "B", "C"]; + const startAt = String(Date.now() + 750); + const children = prefixes.map((prefix) => + Bun.spawn({ + cmd: [process.execPath, script, prefix, startAt], + cwd: root ?? undefined, + env: childEnv, + stdout: "ignore", + stderr: "pipe", + }) + ); + const exits = await Promise.all( + children.map(async (child) => ({ + code: await child.exited, + stderr: await new Response(child.stderr).text(), + })) + ); + for (const exit of exits) { + expect(exit).toEqual({ code: 0, stderr: "" }); + } + + const store = await resolveSecretStore({ projectName: "app" }); + for (const prefix of prefixes) { + for (let index = 0; index < WRITES_PER_PROCESS; index += 1) { + expect(await store.get({ key: `${prefix}_${index}` })).toBe( + `${prefix}-${index}` + ); + } + } + expect(await nonEnvelopeEntries(storeDir)).toEqual([]); +});