Skip to content

fix(native): refuse Hack key files as env sources and match Compose dotenv comments - #849

Merged
roodboi merged 5 commits into
nextfrom
kit/hack-2062-env-file-followups
Nov 15, 2026
Merged

roodboi merged 5 commits into
nextfrom
kit/hack-2062-env-file-followups

Conversation

@roodboi

@roodboi roodboi commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Two follow-ups accepted when #781 (native Compose env files, HACK-2062) merged.

1. Hack key files are never environment sources

A service env_file or the automatic .env is refused with a closed reason, "Hack key and secret-store files cannot be environment sources", before any inspection, cache reuse or read, when it is:

  • named .hack.secret.key at any depth. This covers the checkout key, the shared key under the git common directory and the primary checkout's key;
  • named .hack-secrets.enc.json or .hack-secrets-file.key (the legacy project-relative encrypted store and key);
  • under the Hack home (HACK_HOME, else ~/.hack) when the Hack home lies inside the project root. A Hack home equal to the project root refuses every source.

Paths are compared after Unicode NFC normalization and full Unicode case folding, so a case or normalization alias (Ä and ä, for example) is refused too. An absent optional entry with such a name is refused rather than skipped. The check sits in the shared read of native-compose-environment.ts, which start, preflight, restart preflight and fresh exec all reach through prepareNativeProjectInput. The refusal names only the authored source path.

flowchart TD
  S["env_file entry or automatic .env"] --> L["Literal path admission (inside project root)"]
  L --> K{"Hack key or secret-store file?"}
  K -->|"yes"| R["Closed refusal, nothing read"]
  K -->|"no"| G["Guarded read: ownership, modes, no symlinks, single link"]
  G --> P["Parse and private delivery"]
  class R fail
Loading

Decisions for the lead:

  • The check uses paths only, with no file access. My first version also read the encrypted-store paths from the Hack config and matched files by identity. Running the existing native env suites under strace showed it would make about 19 in-process test files stat and read the real ~/.hack (the base reads nothing there). So the final check is lexical. As a result:
    • It does not cover encrypted stores configured at other paths. That item was not on the accepted list; the default store and key live in the Hack home.
    • It does not recognize a Hack home reachable only through a symlink.
    • Both limits are in the doc.
  • A project inside the Hack home keeps only the name rules. Hack's global service projects live there, so denying the whole Hack home would refuse every source they have.

2. Compose dotenv grammar parity

tests/native-compose-env-parity.test.ts is table-driven: 86 cases, each expectation recorded offline with Docker Compose v5.6.1, docker compose config --format json, with an unreachable DOCKER_HOST. The test itself does not use Docker. The cases cover quoting and escapes, export prefixes, inline comments, multi-line quoted values, empty versus unset, bare names, and the ${VAR:-x}, ${VAR-x}, ${VAR:?err}, ${VAR?err}, ${VAR:+x} and ${VAR+x} operators.

  • Fixed: a tab before # started a comment in native. Compose starts an unquoted inline comment only at a space followed by #, so A=val\t# c is val\t# c in Compose but was val in native. The automatic .env follows the same rule in Compose (checked), so one fix covers both.
  • Documented, not changed: native refuses text after a closing quote (A="v"x); Compose silently drops it. Native also refuses non-identifier keys, as already documented. Both refusals fail closed and are now listed in docs/native/environment-files.md.

Docs: docs/native/environment-files.md (the new rule, a parity section and prerelease change notes), its diagram (.mmd updated and re-rendered with pele-render --theme light, which reproduces the previous SVG byte for byte), and the short v5 page docs/versions/v5/native/preflight.md.

Verification (Linux, pinned Bun 1.4.4)

  • New denylist tests: 10 pass. With the guard disabled, 9 fail (the one pass is the path-only unit test).
  • Parity table: 86 pass. With the old comment rule restored, 3 fail.
  • New CLI case: doctor --native-preflight reports the environment_file_input blocker with the reason, and foreground up exits 1 with only the runtime review call. The key file's content never appears in output or argv.
  • Every suite feat(native): load Compose environment files privately (HACK-2062) #781 touched, plus the new ones (13 files): 578 pass, 7 skip, 0 fail. Run with umask 022; under a 002 umask, 3 existing tests fail on the base as well, because group-writable fixture directories are refused.
  • Root typecheck, Biome on changed files (only existing complexity warnings), docs:prepare + docs:check: pass.

Not verified: macOS, the real-VM env-file scenario (the lead runs it), and the full root suite.

🤖 Generated with Claude Code

roodboi and others added 2 commits November 14, 2026 17:18
A service env_file or the automatic .env is refused with a closed reason
before any inspection or read when it is named like the project key
(.hack.secret.key, at any depth, case-folded and NFC-normalized), a
legacy project-relative store or key, or lies under a Hack home that is
inside the project root. Start, preflight, restart preflight and fresh
exec share the check. The decision uses paths only and opens nothing.

Refs HACK-2062.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A tab before # in an unquoted value is now part of the value, matching
Docker Compose. A table-driven test checks 86 dotenv cases against results
recorded with Docker Compose v5.6.1; the remaining differences are
deliberate refusals and are documented.

Refs HACK-2062.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hack-docs Ready Ready Preview Nov 14, 2026 11:59pm UTC

Request Review

@linear-code

linear-code Bot commented Nov 14, 2026

Copy link
Copy Markdown

HACK-2062

@roodboi

roodboi commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-14T17:18:58.235297Z
We couldn’t complete this review. Comment @codex review to try again.
3bea4a1 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

ASCII-only folding let a non-ASCII case alias of the Hack home (for example
Ä and ä on case-insensitive APFS) through. Fold full Unicode case after NFC
normalization; over-matching only refuses more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Ready for lead: 4cffd62

The #781 follow-ups (HACK-2062), based on next 8e7242db94. Please run the real-VM env-file scenario at this head.

What changed (details in the PR body)

  1. Hack key files are never env sources. A service env_file or the automatic .env is refused with a closed reason before any read when it is named .hack.secret.key at any depth, has a legacy store or key name, or lies under a Hack home that is inside the project root. Paths are compared after NFC normalization and full Unicode case folding. Start, preflight, restart preflight and fresh exec share the check.
  2. Dotenv parity. A table-driven test checks 86 cases against results recorded with Docker Compose v5.6.1. One real difference is fixed: a tab before # no longer starts a comment. Two deliberate refusals are documented: text after a closing quote, and keys that aren't identifiers.

Decisions to confirm

  • The check uses paths only. Reading the Hack config and stat-ing files would have made about 19 existing in-process test files touch the real ~/.hack; I measured this with strace. So stores configured at other paths, and a Hack home reached only through a symlink, are not covered. Both are documented.
  • A project that lies inside the Hack home (where Hack's global service projects live) keeps only the name rules.
  • If the Hack home can't be determined, every env source is refused (fails closed).

What I ran (Linux, pinned Bun 1.4.4, umask 022, temporary TMPDIR, daemon autostart disabled)

  • Every suite feat(native): load Compose environment files privately (HACK-2062) #781 touched, plus the new ones (13 files): 578 pass, 7 skip, 0 fail at 3bea4a136. At this head I reran the denylist, parity and compose-environment suites: 182 pass, 0 fail. With the guards removed, the new tests fail (9 of 10 denylist, 3 of 86 parity).
  • Root typecheck, Biome on changed files (only existing complexity warnings), docs:prepare + docs:check: pass.
  • Codex reviewer (gpt-6.1-sol on codex-dio), one thread. Round 1 had one should-fix: Unicode case aliases such as Ä/ä got through. Fixed in 4cffd6223 with tests. Round 2: CLEAN at 4cffd62. The review was static.
  • computer pr review-status: CLEAR at 4cffd62.

Not verified

  • The Codex bot is at its usage limit, so there is no bot review.
  • Hosted CI was still running when I posted this.
  • Not run: macOS (including real case-insensitive APFS behaviour), the real VM, and the full root suite.
  • Under a 002 umask, 3 existing tests fail on the base as well (group-writable fixture directories are refused). That is an environment effect, not this change.

I will not push to this branch again.

@roodboi

roodboi commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Addendum to the ready note at 4cffd6223e063d2bab9746358724d652ebfbb341 (no push): the GitHub Codex review bot is out of usage. Per the lead, the second-family review of record for this PR is my Codex reviewer (gpt-6.1-sol on codex-dio), which returned CLEAN at 4cffd6223e063d2bab9746358724d652ebfbb341.

The privacy check reads /home/ segments as possible personal paths. Rename
the synthetic Hack home fixtures; the assertions are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Ready for lead: c2f11e7

This replaces the ready note at 4cffd6223e. I pushed after it because hosted CI found a real gate failure in my change.

  • What failed: test shard 8/8 stopped at privacy:check, which flagged /home/ in four synthetic fixture paths in tests/native-env-source-denylist.test.ts. I had not run privacy:check locally before the first note.
  • Fix: c2f11e723 renames those fixtures (state/hack-state, /fixture/other/.hack, /fixture/outside/.hack). It is test-only; the assertions and product source are unchanged.
  • Checks at this head: privacy:check ok, full bun run check (Bun pin, privacy, lint) passes, and the denylist tests pass 10 of 10. Everything else in the previous note still holds: the same source, and the 13-suite run of 578 pass at 3bea4a136 plus reruns.
  • Second-family review of record: my Codex reviewer (gpt-6.1-sol on codex-dio), the same thread for all three rounds, is CLEAN at c2f11e7. It confirmed the inside-root and outside-root Hack home cases still test what they claim. The GitHub Codex bot is out of usage.
  • Hosted CI at this head was still running when I posted this. Not verified: macOS and the real VM; those are in your Mac batch.

I will not push to this branch again.

@roodboi

roodboi commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Mac batch result at c2f11e7: one assertion conflicts with #794.

On this head alone tests/native-preflight-cli.test.ts passes 35 of 35 on the Mac. In the combined tree with #794 (first-use runtime setup, merging first) the test "preflight and start refuse the project key as an env file before reading it" fails at line 330: it requires every call after the refusal to be project plan, and with #794 up also makes the read-only runtime setup inspection call. The refusal text, exit code and privacy assertions still pass.

Wanted: accept project plan and runtime setup there, keep it strict against any pool, file-delivery or start call. The real-VM env-file scenario result for the combined tree follows in the batch comment.

🤖 Generated with Claude Code

…ey-source refusal

With first-use setup guidance (#794), native start also runs
'runtime setup inspect --json' before refusing. The test now accepts only
'project plan' and that read-only inspection after start, and still fails
on any other runtime call such as a pool, file-delivery or start request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

Ready for lead: e1dec86

This replaces the note at c2f11e7238. It is a test-only change for the combined-tree result you posted.

  • Change: in "preflight and start refuse the project key as an env file before reading it", the check after up now accepts only project plan … and the read-only runtime setup inspect (flags limited to --json and --quiescence). Any other runtime call fails the test: a pool, file-delivery or start call, or a mutating runtime setup. Two self-checks in the test confirm that runtime setup --no-interactive --json and project start are rejected. The refusal text, exit code and privacy assertions are unchanged.
  • Verified locally (Linux):
    • tests/native-preflight-cli.test.ts passes 35 of 35 at this head.
    • It also passes 35 of 35 in a local merge of this head with feat(native): guide first-use runtime setup #794 (3cda82bd4).
    • In that merged tree, the previous assertion fails, which reproduces your Mac result.
    • Full bun run check passes.
  • Order: no follow-up needed. The refusal still comes before any effect, and both earlier calls are read-only. One observation for the feat(native): guide first-use runtime setup #794 owner, not a request: if the runtime isn't set up yet, up may report "setup required" before this env-file refusal. hack doctor --native-preflight lists both at once.
  • Second-family review of record: my Codex reviewer (gpt-6.1-sol on codex-dio), the same thread for all four rounds, is CLEAN at e1dec86. It confirmed the allowlist still rejects pool, delivery, start and mutating setup calls. The GitHub Codex bot is out of usage.
  • Hosted CI at this head was still running when I posted this. Not verified by me: macOS and the real VM.

I will not push to this branch again.

@roodboi
roodboi merged commit 7a6f26f into next Nov 15, 2026
21 checks passed
@roodboi
roodboi deleted the kit/hack-2062-env-file-followups branch November 15, 2026 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant