fix(env): keep stored ciphertext when a secret value is unchanged

.hack/.gitignore

@@ -5,5 +5,6 @@
 .env.state.json
 hack.env.local.yaml
 hack.env.*.local.yaml
+.hack.env*.tmp
 tickets/
 # end managed by hack

docs/env.md

@@ -51,6 +51,7 @@ worktrees inherit the rules with zero setup. It covers (patterns relative to
 - `.env.state.json`
 - `hack.env.local.yaml`
 - `hack.env.*.local.yaml`
+- `.hack.env*.tmp` (staging files an interrupted env write can leave behind)
 - `tickets/` (retained only to contain retired machine-local ticket caches during upgrades)
 
 How it is maintained:
@@ -412,6 +413,37 @@ input and without writing anything:
 `.hack/hack.env.<overlay>.local.yaml`) instead of the shared repo env file — use it for
 per-checkout values you don't want to commit.
 
+Repeating `hack env add` or `hack env set` with the value already stored is a no-op: when
+the existing secret decrypts to the same value under the current key, its ciphertext is
+kept, so once Hack has written the file it stays byte-identical and Git shows no change.
+A hand-edited file (comments, other formatting, or no `global: {}`) is normalized on the
+first repeat and stays byte-identical after that. A different value, a plain entry
+becoming secret, or an entry that does not decrypt under the current key gets new
+ciphertext with a fresh IV. Each command writes one key; a script that sets several keys
+is not atomic across them.
+
+Each env file write goes to a hidden `.<file>.<id>.tmp` file in the same directory and
+replaces the file by rename, so an interrupted write leaves the previous file in place.
+A write killed before the rename can leave that staging file behind. It holds the whole
+new layer, so `.hack/.gitignore` ignores `.hack.env*.tmp`, and you can delete it. Hack
+does not remove it on the next write, because another write may still be using it. The
+rename has these effects:
+
+- a symlinked env file keeps its link; the new contents land at the file the link points
+  to, and the staging file is created next to that file
+- a symlink whose target does not exist is replaced by a regular file
+- a hard link is broken: the env file gets a new inode, and any other name for the old
+  file keeps the previous contents
+- the directory must be writable, even when the env file itself is writable
+
+A rename does not need write access to the file it replaces, so Hack checks that access
+itself before it stages anything. It refuses to replace an existing env file that it
+cannot read, that is not a regular file, that is not writable, or whose owner and group
+it cannot keep (for example a root-owned file left by an earlier `sudo hack`). The error
+names the file and the reason, never a value, and the file is left as it was. Fix the
+file's permissions or ownership and run the command again. A rewrite keeps the file's
+permission bits, owner and group.
+
 If you omit `key`, `value`, or `--service`, `hack env add` prompts for them interactively.
 Pass `--no-interactive` (or set `HACK_NO_INTERACTIVE=1`) for scripted/agent usage: the
 command applies documented defaults or fails fast with `E_INTERACTIVE_REQUIRED` instead of

plugins/hack/skills/hack-cli/references/patterns.md

@@ -14,7 +14,7 @@ missing part with ad hoc scripts.
 | HACK-P05 | Branch-owned external routing claim | Deferred | WU29 |
 | HACK-P06 | Opt-in public tunnel with end-to-end readiness | Deferred | WU29 |
 | HACK-P07 | Laptop and headless-runner capability overlays | Partial | WU27/WU30 |
-| HACK-P08 | Tracked defaults plus idempotent machine-local hydration | Deferred | WU27/WU14 |
+| HACK-P08 | Tracked defaults plus idempotent machine-local hydration | Partial | WU27/WU14 |
 | HACK-P09 | Disposable run workspaces with reference-aware retention | Partial | WU12/WU30 |
 | HACK-P10 | Pinned tool image plus independent dependency caches | Partial | WU30 |
 | HACK-P11 | Stable agent guidance and typed operation results | Partial | WU19/WU21 |
@@ -135,12 +135,12 @@ missing part with ad hoc scripts.
 ## HACK-P08: Tracked defaults plus idempotent machine-local hydration
 
 - Use when: A repository tracks env defaults and each machine adds local values on top.
-- Disposition: Deferred. Missing: Hack has no semantic no-op detection or atomic machine-local import contract for env hydration.
+- Disposition: Partial. Missing: No-op detection and rename-committed writes cover one key per command. Hack has no multi-key import: an interrupted hydration can leave some keys updated and others not.
 - Owner: WU27/WU14; tracked in HACK-1152.
-- Commands: none yet; do not substitute a hand-written recipe.
+- Commands: `hack env set <key> --local --value-stdin --secret`, `hack env explain <key> --json`
 - Prerequisites: Tracked defaults, a machine-local layer and preserved operator overrides.
-- Env ownership: Defaults are tracked; hydration writes only the local layer and keeps values out of argv.
-- Readiness: Repeat hydration leaves the tracked tree clean; an interrupted import still leaves a resolvable layer.
+- Env ownership: Defaults are tracked; hydration runs `hack env set <key> --local --value-stdin --secret` once per key with the value piped on stdin, so values stay out of argv. Without `--local` the same command writes a tracked default.
+- Readiness: Setting a secret to its current value keeps the stored ciphertext, so repeat hydration leaves the tracked tree clean; a changed value gets new ciphertext with a fresh IV. Each layer write commits by rename, so an interrupted write leaves the previous layer resolvable.
 - Cleanup: Unchanged values cause no rewrite.
 - Provenance: single-consumer generalization (HACK-P08 research register).
 
@@ -214,4 +214,4 @@ missing part with ad hoc scripts.
 
 ---
 
-Content revision: `03a667cfa6a4`. Generated by Hack; edit the canonical source and regenerate the plugin.
+Content revision: `247c8f20c3c8`. Generated by Hack; edit the canonical source and regenerate the plugin.

src/agents/pattern-catalogue.ts

@@ -270,20 +270,23 @@ export const PATTERN_CATALOGUE: readonly PatternCard[] = [
     title: "Tracked defaults plus idempotent machine-local hydration",
     useWhen:
       "A repository tracks env defaults and each machine adds local values on top.",
-    disposition: "deferred",
-    supportedCommands: [],
+    disposition: "partial",
+    supportedCommands: [
+      "hack env set <key> --local --value-stdin --secret",
+      "hack env explain <key> --json",
+    ],
     prerequisites:
       "Tracked defaults, a machine-local layer and preserved operator overrides.",
     envOwnership:
-      "Defaults are tracked; hydration writes only the local layer and keeps values out of argv.",
+      "Defaults are tracked; hydration runs `hack env set <key> --local --value-stdin --secret` once per key with the value piped on stdin, so values stay out of argv. Without `--local` the same command writes a tracked default.",
     readiness:
-      "Repeat hydration leaves the tracked tree clean; an interrupted import still leaves a resolvable layer.",
+      "Setting a secret to its current value keeps the stored ciphertext, so repeat hydration leaves the tracked tree clean; a changed value gets new ciphertext with a fresh IV. Each layer write commits by rename, so an interrupted write leaves the previous layer resolvable.",
     cleanup: "Unchanged values cause no rewrite.",
     negativeControl:
       "Re-encrypting unchanged entries causes diff and stash churn, and values passed in command arguments are not a secure import mechanism.",
     domainOwner: "WU27/WU14",
     missingContract:
-      "Hack has no semantic no-op detection or atomic machine-local import contract for env hydration.",
+      "No-op detection and rename-committed writes cover one key per command. Hack has no multi-key import: an interrupted hydration can leave some keys updated and others not.",
     provenance: { consumers: 1, source: "HACK-P08 research register" },
     example: null,
     e2eScenario: null,

src/commands/doctor.ts

@@ -2999,7 +2999,8 @@ async function checkProjectEnvKey({
 /**
  * Warns when hack-generated machine-local files (`.hack/.internal/`,
  * `.hack/.branch/`, `.hack/.env`, `.hack/.env.state.json`,
- * `.hack/hack.env.*.local.yaml`, `.hack.secret.key`) are tracked in git.
+ * `.hack/hack.env.*.local.yaml`, `.hack/tickets`, `.hack/.hack.env*.tmp`
+ * staging files, `.hack.secret.key`) are tracked in git.
  * A tracked secret key is reported as an error because it is a committed
  * secret. `hack doctor --fix` untracks the offenders (files stay on disk) and
  * ensures the committed `.hack/.gitignore`.

src/lib/doctor-generated-files.ts

@@ -28,6 +28,7 @@ export function buildGeneratedFilePathspecs(opts: {
     `${dir}/.env.state.json`,
     `${dir}/tickets`,
     `${dir}/hack.env.*.local.yaml`,
+    `${dir}/.hack.env*.tmp`,
     PROJECT_ENV_KEY_FILENAME,
   ];
 }

src/lib/fs.ts

@@ -1,4 +1,17 @@
-import { lstat, mkdir, stat } from "node:fs/promises";
+import { randomUUID } from "node:crypto";
+import {
+  access,
+  constants,
+  type FileHandle,
+  lstat,
+  mkdir,
+  open,
+  realpath,
+  rename,
+  stat,
+  unlink,
+} from "node:fs/promises";
+import { basename, dirname, join } from "node:path";
 
 export async function pathExists(absolutePath: string): Promise<boolean> {
   try {
@@ -89,6 +102,212 @@ export async function writeTextFileIfChanged(
   return { changed: true };
 }
 
+/**
+ * Replaces `absolutePath` with `content` only when the text differs, publishing
+ * the new contents atomically.
+ *
+ * Commit point: a uniquely named sibling is created exclusively (no symlink
+ * follow), written and fsynced, then renamed over the target. A process that
+ * dies before the rename leaves the previous complete file in place and at most
+ * an orphaned `.<name>.<uuid>.tmp` sibling that no reader selects; the staging
+ * file is removed on every failure this process observes. Orphans are never
+ * swept here, because a concurrent writer may still own one; callers that write
+ * under `.hack/` rely on its managed `.gitignore` to keep them unstaged.
+ * Directory fsync is best effort.
+ *
+ * Rename needs no write permission on the file it replaces, so the guards an
+ * in-place write got from the kernel are checked explicitly, before any staging
+ * file exists, and fail with {@link AtomicWriteRefusedError}: an existing target
+ * that cannot be read, is not a regular file or is not writable is refused, and
+ * so is one whose owner and group the staging file cannot take (for example a
+ * root-owned file left by `sudo hack`). A caller that merged a change into what
+ * it read therefore never replaces contents it could not see. These checks are
+ * not an atomic fence against a concurrent permission change.
+ *
+ * An existing target keeps its permission bits, owner and group, and an
+ * existing symlink keeps pointing where it did: the replacement lands at the
+ * link's resolved path, as a plain write through the link would. A new file
+ * gets the umask default. Unlike an in-place write, a dangling symlink is
+ * replaced by a regular file, a hard link to the target is detached (its other
+ * names keep the previous contents), and the containing directory must be
+ * writable.
+ */
+export async function writeTextFileAtomicIfChanged(
+  absolutePath: string,
+  content: string
+): Promise<{ readonly changed: boolean }> {
+  const existing = await inspectAtomicWriteTarget(absolutePath);
+  if (existing.kind === "existing" && existing.text === content) {
+    return { changed: false };
+  }
+  const target =
+    existing.kind === "absent" ? absolutePath : await realpath(absolutePath);
+  if (existing.kind === "existing") {
+    try {
+      await access(target, constants.W_OK);
+    } catch {
+      throw new AtomicWriteRefusedError({
+        path: absolutePath,
+        reason: "not_writable",
+      });
+    }
+  }
+  const staging = join(
+    dirname(target),
+    `.${basename(target)}.${randomUUID()}.tmp`
+  );
+  const file = await open(
+    staging,
+    constants.O_CREAT |
+      constants.O_EXCL |
+      constants.O_NOFOLLOW |
+      constants.O_WRONLY,
+    0o666
+  );
+  let committed = false;
+  try {
+    if (existing.kind === "existing") {
+      // chown can clear set-id bits, so the mode is applied after it.
+      await preserveOwnership({ file, existing, path: absolutePath });
+      await file.chmod(existing.mode);
+    }
+    await file.writeFile(content, "utf8");
+    await file.sync();
+    await rename(staging, target);
+    committed = true;
+  } finally {
+    await file.close();
+    if (!committed) {
+      await unlink(staging).catch(() => undefined);
+    }
+  }
+  await syncDirectoryBestEffort(dirname(target));
+  return { changed: true };
+}
+
+/** Why {@link writeTextFileAtomicIfChanged} refused to replace a file. */
+export type AtomicWriteRefusal =
+  | "unreadable"
+  | "not_regular_file"
+  | "not_writable"
+  | "owner_not_preserved";
+
+const ATOMIC_WRITE_REFUSAL_MESSAGES: Readonly<
+  Record<AtomicWriteRefusal, string>
+> = {
+  unreadable: "it exists but cannot be read, so its contents cannot be kept",
+  not_regular_file: "it is not a regular file",
+  not_writable: "it is not writable",
+  owner_not_preserved: "its owner and group cannot be preserved",
+};
+
+/**
+ * Closed refusal from {@link writeTextFileAtomicIfChanged}. The message names
+ * the path and a fixed reason only; it never carries file contents or a cause.
+ */
+export class AtomicWriteRefusedError extends Error {
+  readonly path: string;
+  readonly reason: AtomicWriteRefusal;
+
+  constructor(opts: {
+    readonly path: string;
+    readonly reason: AtomicWriteRefusal;
+  }) {
+    super(
+      `Refusing to replace ${opts.path}: ${ATOMIC_WRITE_REFUSAL_MESSAGES[opts.reason]}.`
+    );
+    this.name = "AtomicWriteRefusedError";
+    this.path = opts.path;
+    this.reason = opts.reason;
+  }
+}
+
+type AtomicWriteTarget =
+  | { readonly kind: "absent" }
+  | {
+      readonly kind: "existing";
+      readonly text: string;
+      readonly mode: number;
+      readonly uid: number;
+      readonly gid: number;
+    };
+
+/**
+ * Classifies the current target without collapsing "exists but unreadable"
+ * into "absent". Only a missing path, or a symlink whose target is missing, is
+ * `absent`; any other lstat, stat or read failure is a refusal.
+ */
+async function inspectAtomicWriteTarget(
+  absolutePath: string
+): Promise<AtomicWriteTarget> {
+  const refuse = (reason: AtomicWriteRefusal) =>
+    new AtomicWriteRefusedError({ path: absolutePath, reason });
+  try {
+    await lstat(absolutePath);
+  } catch (error: unknown) {
+    if (isMissingPathError(error)) {
+      return { kind: "absent" };
+    }
+    throw refuse("unreadable");
+  }
+  let info: Awaited<ReturnType<typeof stat>>;
+  try {
+    info = await stat(absolutePath);
+  } catch (error: unknown) {
+    // lstat found an entry, so a missing stat target is a dangling symlink.
+    if (isMissingPathError(error)) {
+      return { kind: "absent" };
+    }
+    throw refuse("unreadable");
+  }
+  if (!info.isFile()) {
+    throw refuse("not_regular_file");
+  }
+  const text = await readTextFile(absolutePath);
+  if (text === null) {
+    throw refuse("unreadable");
+  }
+  return {
+    kind: "existing",
+    text,
+    mode: Number(info.mode) & 0o7777,
+    uid: Number(info.uid),
+    gid: Number(info.gid),
+  };
+}
+
+/** Gives the staging file the target's owner and group, or refuses. */
+async function preserveOwnership(opts: {
+  readonly file: FileHandle;
+  readonly existing: { readonly uid: number; readonly gid: number };
+  readonly path: string;
+}): Promise<void> {
+  const staged = await opts.file.stat();
+  if (staged.uid === opts.existing.uid && staged.gid === opts.existing.gid) {
+    return;
+  }
+  try {
+    await opts.file.chown(opts.existing.uid, opts.existing.gid);
+  } catch {
+    throw new AtomicWriteRefusedError({
+      path: opts.path,
+      reason: "owner_not_preserved",
+    });
+  }
+}
+
+async function syncDirectoryBestEffort(directory: string): Promise<void> {
+  let handle: FileHandle | null = null;
+  try {
+    handle = await open(directory, constants.O_RDONLY);
+    await handle.sync();
+  } catch {
+    // Some filesystems refuse directory fsync; the file itself is synced.
+  } finally {
+    await handle?.close().catch(() => undefined);
+  }
+}
+
 /**
  * Ensures a marker-delimited managed block exists in a gitignore-style file.
  *