Repository navigation
Conversation
setProjectEnvValue now keeps an entry's existing ciphertext when it already decrypts to the requested value under the current key, so repeating `hack env set --secret --value-stdin` with the same value leaves the tracked env file byte-identical and git status clean. A different value, a plain entry becoming secret, or an entry that does not open under the current key is re-encrypted with a fresh IV. Env layer writes now commit by rename from an exclusive, fsynced sibling, so a write interrupted before the commit leaves the previous layer intact and resolvable. Existing permission bits and symlink targets are preserved. HACK-P08 moves from deferred to partial: per-key no-op detection and atomic layer writes exist; a multi-key hydration import remains missing. Refs HACK-1152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…vior A `--local` env write killed before its commit rename left a `.hack.env*.<uuid>.tmp` staging file that git did not ignore, so a later `git add -A` could commit machine-local values, and a tracked-layer orphan left the tree dirty. The managed `.hack/.gitignore` now ignores `.hack.env*.tmp`, and `hack doctor` flags a tracked staging file. Tests now cover symlink preservation, staging cleanup after a failed rename, and clean git status after a killed tracked or local write. The argv-env-value recipe skips flags, so the P08 card can name `hack env set <key> --local --value-stdin --secret`. Docs describe the first-repeat normalization of hand-edited files and the rename edge cases. Refs HACK-1152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refs HACK-1152 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
With stable ciphertext, setting an unchanged secret keeps the stored ciphertext, so the shared cipher fixture returned identical values and the ciphertext-only change tests no longer produced a change. Encrypt each fixture value in its own store so every call is a fresh encryption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rename-committed env write treated any read failure as an absent file, so `hack env set` on a layer it could not read (mode 000, or root-owned 0600 from an earlier `sudo hack`) built a layer holding only the new key and renamed it over the old one, dropping every other value. Rename also skips the write-permission check an in-place write got, so a read-only or foreign layer was silently replaced with a new owner. writeTextFileAtomicIfChanged now classifies the target before staging: only a missing path or a dangling symlink is absent. An existing target that cannot be read, is not a regular file, is not writable, or whose owner and group the staging file cannot take is refused with a closed, value-free AtomicWriteRefusedError, and the file is left as it was. A rewrite keeps the file's owner and group as well as its mode. Document the refusals and that rename detaches hard links, and list the staging pathspec in the doctor check docstring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve plugins/hack/skills/hack-cli/references/patterns.md by regenerating it (bun run generate:agent-plugins) from the merged src/agents/pattern-catalogue.ts, carrying this branch's pattern wording and the changes merged in train b18. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve generated plugin conflicts (plugins/hack/skills/hack-cli/references/patterns.md ) by regenerating them with bun run generate:agent-plugins from the merged canonical source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear: HACK-1152 (WU18-P08, pattern HACK-P08)
Final behavior
setProjectEnvValue(the path behindhack env add/hack env set) keeps a secret entry's stored ciphertext when that ciphertext decrypts to exactly the requested value under the current key. Repeatinghack env set KEY --secret --value-stdinwith the same value leaves the env file byte-identical, reportsNo changes needed, and leavesgit statusclean.writeProjectEnvConfigFile, used by set, unset and migrate) commit by rename.writeTextFileAtomicIfChangedinsrc/lib/fs.tscreates an exclusive sibling (O_CREAT|O_EXCL|O_NOFOLLOW), writes and fsyncs it, then renames it over the target. If the process dies before the rename, the previous complete layer stays in place and still resolves. The helper keeps the file's permission bits, an existing symlink keeps its link (the write lands at the resolved target), and the staging file is removed on any failure the process sees..<layer>.<uuid>.tmpstaging file that holds the whole layer. The managed.hack/.gitignoreblock now ignores.hack.env*.tmp, so an orphan from a--localwrite cannot be committed bygit add -A, and an orphan from a tracked-layer write does not leavegit statusdirty. Existing projects pick up the entry on their next--localwrite,hack up,hack initorhack doctor --fix.hack doctor's generated-files check also flags a tracked staging file. Orphans are not swept on the next write, because a concurrent writer may still own one. This repository's own.hack/.gitignoreblock is refreshed.deferredtopartial. It now names the hydration command directly:hack env set <key> --local --value-stdin --secret, next tohack env explain <key> --json. To allow that, theargv-env-valuebanned-recipe scan intests/helpers/pattern-recipes.tsskips flags and stops at an inline-code backtick. It still flags any form with two operands, includinghack env set --local KEY value, a backtick command substitution as the value, and a value-taking flag such as--env dev. The missing contract is narrowed: each command writes one key, and there is no multi-key import.docs/env.mddocuments the no-op, rotation and rename behavior. It now says the file stays byte-identical once Hack has written it, because a hand-edited layer is normalized on the first repeat. It also lists the rename edge cases: a dangling symlink becomes a regular file, and the directory must be writable even when the file is. The ignore-rules list includes the new entry.Validation (local, macOS host, head
ad65a7ea)tests/project-env-stable-ciphertext.test.ts: 10 pass. New in this rework:--locallayer. Each asserts that the old bytes are kept and resolve, thatgit status --porcelainis empty with the orphan present, and thatgit add -Astages nothing.tests/hack-gitignore.test.tschecks the ignore rule with realgit check-ignorefor tracked- and local-layer staging names.tests/doctor-generated-files.test.tscovers detection and untracking of a leaked staging file, with a clean status afterwards.tests/pattern-catalogue.test.tshas a new allowed/flagged table forargv-env-value.bun run generate:agent-pluginsregenerated onlyplugins/hack/skills/hack-cli/references/patterns.md. No ownership fingerprint or instruction revision change was required.bun run --cwd packages/cli typecheck: clean.bun run --cwd packages/cli check: 0 errors (34 pre-existing warnings, none in touched files). Biome on the 10 changed source/test files: clean.bun run privacy:check: ok. No CLI surface or Rust changes.privateCliEnv. The key file lives in the temp project. Nothing touches Docker, the keychain, the real HOME or client config.Negative controls (mutations; each reverted, tree verified clean)
Bun.writepath: the SIGKILL-before-commit tests fail.realpath(verifier M7, previously survived): the symlink test fails.unlinkon failure (verifier M8, previously survived): the staging-cleanup test fails..hack.env*.tmpfrom the managed entries: both kill tests, the check-ignore test and the doctor untrack test fail (4 fail).argv-env-valueregex: the rendered-catalogue scan and the new allowed/flagged table fail.Remaining gaps
partial(owner WU27/WU14)..hack/, its staging file lands next to that target, where the managed ignore does not apply..envand secret-storeenv setpaths are unchanged. The one-time legacy migration (buildMigratedValues) still re-encrypts.Rework 2
Base: remote head
52f0ffc9; pushed head5d58e9b0(includes the main session's fixture fix). New commit:fix(env): refuse to replace an env layer that cannot be read or written.Fixed
writeTextFileAtomicIfChangedused to treat any read error as "file absent".hack env seton a layer it could not read (mode 000, or root-owned 0600 aftersudo hack) then wrote a layer holding only the new key and renamed it over the old one. Now the helper checks the target before it stages anything. Only a missing path, or a symlink whose target is missing, counts as absent. An existing target that cannot be read, is not a regular file, or is not writable (access(W_OK)) is refused. So is one whose owner and group the staging file cannot take. The error is a closedAtomicWriteRefusedError: it gives the path and a fixed reason, never a value, and the file is left as it was. This brings back the guard the old in-place write got from the kernel.fchowned to the old owner and group before the rename. If it cannot be, the write is refused. As a result,sudo hack env setkeeps the user's ownership instead of leaving a root-owned layer.docs/env.mdnow covers the refusals and says that rename breaks hard links. ThecheckTrackedGeneratedFilesdocstring now lists the.hack/.hack.env*.tmppathspec (and.hack/tickets).Tests (
tests/project-env-stable-ciphertext.test.ts, 16 pass)New tests:
unreadable), checked through the library and through the CLI (env set --secret --value-stdinexits non-zero). The bytes are kept and no staging file is left.not_writable). The inode, mode and bytes are kept.not_regular_file) before anything is staged.tests/fixtures/atomic-write-rename-failure-worker.ts) whose commit rename fails after staging. It asserts that the old bytes are kept and no staging file is left.The permission refusals are skipped when running as root, because root ignores permission bits.
Mutations (each reverted;
src/lib/fs.tsbyte-compared with the good copy afterwards):access(W_OK)check removed: the read-only test fails.unlinkon failure removed: the rename-failure test fails.Gates on the new head
--log-opts=origin/next..HEAD: no leaks (5 commits).bun run --cwd packages/cli typecheck: clean.check: 0 errors. Biome on the changed files: clean.bun run test(local macOS, heavily loaded host, afterbun run build:config-compiler): 7203 pass, 126 skip, 35 fail. I reran each failing file serially with--timeout 60000:AtomicWriteRefusedError. I ran five of these files with the pre-changesrc/lib/fs.tsfrom52f0ffc9, and they failed the same way. So these are host-environment failures on this machine, not regressions. Hostedtestpassed on52f0ffc9.Still open
owner_not_preservedrefusal path cannot be reached by a non-root test user, so no test covers it. Covering it needs a root CI job or a mockedfchown..hack/for symlinked layers, the lack of multi-key atomicity, and best-effort directory fsync.🤖 Generated with Claude Code