Skip to content

fix(utils): reject a project-.env config directory override - #3295

Merged
Yeachan-Heo merged 1 commit into
Yeachan-Heo:devfrom
10kH:fix/config-dir-trust
Jul 27, 2026
Merged

fix(utils): reject a project-.env config directory override#3295
Yeachan-Heo merged 1 commit into
Yeachan-Heo:devfrom
10kH:fix/config-dir-trust

Conversation

@10kH

@10kH 10kH commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Stacked on #3285 — it contains the parent commit and the leaf env-file module this needs. Review/merge #3285 first; this branch is #3285 plus one commit.

The other half of the same escalation

#3285 closed GJC_CODING_AGENT_DIR. The same escalation is reachable through the config-directory name, which I deferred there to avoid conflicting with the already-approved #3267.

getConfigDirName() read GJC_CONFIG_DIR / PI_CONFIG_DIR straight from process.env, and DirResolver joins that name with the home directory to build configRoot (dirs.ts:185), with the agent directory beneath it (:187). Both of those supply .env files that $credentialEnv treats as trusted — so this one name was also enough to make a repository's own .env trusted, recovering every endpoint and credential redirect the boundary rejects.

Measured on this source — repo/.env sets GJC_CONFIG_DIR=../repo/.evil, repo/.evil/.env sets a provider base URL:

before: configRoot = /tmp/_cfg2/repo/.evil
        $credentialEnv("ANTHROPIC_BASE_URL") = "https://attacker.example"
after:  configRoot = /tmp/_cfg2/home/.gjc
        $credentialEnv("ANTHROPIC_BASE_URL") = undefined

Operator path verified in the same shape: GJC_CONFIG_DIR=.custom from the shell still lands configRoot at <home>/.custom and that directory's .env resolves.

Fix

The same conservative ambiguity rule the parent commit applies to GJC_CODING_AGENT_DIR, evaluated per key so a legacy PI_CONFIG_DIR still applies when the primary one is rejected. It uses the leaf env-file module from #3285 — importing parseEnvFile from env.ts directly would be a cycle, and I confirmed that empirically: the check silently never fires because dirs.ts runs while env.ts is still initialising.

Relationship to #3267

Complementary, not overlapping. #3267 constrains the shape of the name (no .. escaping home); this constrains its source. Both are needed — shape alone still allows a repository that lives under $HOME to point the config root at itself without any .., and source alone still allows a trusted-but-malformed value. They touch the same function, so whichever lands second will need a trivial rebase; happy to do that.

Tests

Extends agent-dir-trust.test.ts (now 5 cases): a planted config directory is ignored, and a config directory inherited from the launching shell still supplies its .env.

Proof-first: reverting the config-dir check fails exactly the planted case (4 pass / 1 fail).

Verification

bun run check in packages/utils: exit 0. packages/utils/test: 218 pass / 0 fail. coding-agent discovery suites: 130 pass / 0 fail.

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE — MERGE_READY_WITH_ORDER (parent-first hold) — exact head 162127f3a4a43db93de146419cdf18b7759ebe73

This PR is not an independent clean-base change; it is a two-commit stack (#3285's exact head e88771a26cd4bbc3215239973a81fdeba6641244 + one new commit) and must be evaluated and merged as such.

Parent/head decomposition — verified directly against git, not the PR description:

git log --oneline pr3295
162127f3a fix(utils): reject a project-.env config directory override   <- head, this PR's new commit
e88771a26 fix(utils): reject a project-.env agent directory override   <- == #3285 exact head, byte-identical
18f6935b7 fix(coding-agent): repair dev CI 30244279756 after #3257 (#3275)

pr3295^1 == e88771a26cd4bbc3215239973a81fdeba6641244 exactly (confirmed via git rev-parse), and git merge-base pr3295 origin/dev == 18f6935b7, i.e. this stack branches from the same pre-#3281 base as #3285 with no drift introduced by the second commit. The incremental diff (e88771a26..162127f3a) touches only packages/utils/{CHANGELOG.md,src/dirs.ts,test/agent-dir-trust.test.ts} — no re-touch of anything #3285 already changed, confirming this really is "#3285 plus one commit" and not a rebase-and-diverge.

Checks performed on the incremental commit only (the parent portion was already reviewed and approved under #3285):

  • Source-trust check for GJC_CONFIG_DIR/PI_CONFIG_DIR: getConfigDirName() moved from raw process.env.GJC_CONFIG_DIR ?? process.env.PI_CONFIG_DIR ?? CONFIG_DIR_NAME to trustedConfigDirName("GJC_CONFIG_DIR") ?? trustedConfigDirName("PI_CONFIG_DIR") ?? CONFIG_DIR_NAME. Both names are now rejected when their value exactly matches what the project .env sets, mirroring trustedAgentDirOverride's rule from the parent commit exactly — same conservative ambiguity trade-off (operator whose environment happens to carry the identical value loses the override), consistently applied.
  • Per-key fallback: verified trustedConfigDirName is called independently per name (GJC_CONFIG_DIR first, then PI_CONFIG_DIR, then the CONFIG_DIR_NAME default) — undefined-propagation semantics of the original ?? chain are preserved, and a legacy PI_CONFIG_DIR correctly still applies if GJC_CONFIG_DIR specifically gets rejected as project-planted (not both names collapsed together).
  • Interaction with #3267 shape containment: confirmed complementary, not overlapping, by reasoning through the two independent failure modes — #3267 constrains the shape of the config-dir name (no .. escaping home) but has no source check, so a value that is well-formed and even lives entirely under $HOME would still pass #3267 unchecked. This PR constrains the source (project .env cannot supply the value at all) regardless of shape, so it also closes the case #3267 alone cannot: a malicious directory the shape check would accept because it never needs .. to begin with. Both are genuinely necessary; this is not a duplicate of #3267's concern.
  • cwd-under-HOME attack: the source-equality check in trustedConfigDirName has no path-shape dependency — it rejects purely on project-.env provenance, so it equally covers a malicious config directory placed inside $HOME (no .. needed) as it does one placed outside. Confirmed this is not merely incidental: the check never inspects the resolved path's relationship to $HOME, only whether the raw value's source is the project .env.
  • Cycle avoidance via env-file leaf module: trustedConfigDirName reuses the parseEnvFile import already added to dirs.ts by the parent commit (./env-file, not ./env) — no new import needed, no cycle reintroduced. The PR body's claim that importing from env.ts directly would silently never fire (because dirs.ts runs while env.ts is still initializing) is consistent with the same reasoning already validated in #3285's review.
  • Tests: extends the same agent-dir-trust.test.ts suite (now 5 cases) rather than creating a parallel file — correct, since it is testing the same trust boundary. New cases: a config directory planted by the project .env is ignored (probed via a .env inside the config dir, matching the parent's probe pattern), and a config directory inherited from the launching shell (GJC_CONFIG_DIR=.custom relative to a controlled HOME) still resolves. Test hermeticity extended correctly: GJC_CONFIG_DIR/PI_CONFIG_DIR are now also deleted from the child env alongside the existing GJC_CODING_AGENT_DIR cleanup.
  • CI: all 23 checks pass on the exact head (test:packages/utils/test/agent-dir-trust.test.ts explicitly green; native-build, darwin-arm64 smoke, install-methods, and both ts-build jobs all completed pass after an initial pending window).
  • mergeStateStatus: reported UNSTABLE by the GitHub API at time of review — consistent with this being a stacked/dependent branch on an unmerged parent, not a defect in the diff itself.
  • No automated reviewer comments present.

Required merge order: #3285 MUST land first. This PR's base commit is #3285's exact head verbatim; merging #3295 before #3285 is not meaningful (there is no independent diff to land) and merging in the other order requires no rebase at all since the stack is already linear against #3285. If #3285 is rebased or its commit hash changes before merge, this stack needs a rebase to match.

No merge action taken by me on either PR — dev CI on the current base is nonterminal (per the standing instruction not to merge while post-#3250 dev CI runs), and this is a signed review verdict only.


[repo owner's gaebal-gajae (clawdbot) 🦞]

Same escalation as the agent-directory override in the parent commit, through
the other name. getConfigDirName() read GJC_CONFIG_DIR / PI_CONFIG_DIR straight
from process.env, and DirResolver joins that name with the home directory to
build configRoot (dirs.ts:185), with the agent directory beneath it. Both of
those supply .env files that $credentialEnv treats as trusted.

Measured on this source, with repo/.env setting GJC_CONFIG_DIR=../repo/.evil and
repo/.evil/.env setting a provider base URL:

  before: configRoot = /tmp/_cfg2/repo/.evil
          $credentialEnv("ANTHROPIC_BASE_URL") = "https://attacker.example"
  after:  configRoot = /tmp/_cfg2/home/.gjc
          $credentialEnv("ANTHROPIC_BASE_URL") = undefined

An operator setting GJC_CONFIG_DIR from their shell still works, verified in the
same shape: configRoot lands at <home>/.custom and that directory's .env
resolves.

Applies the same conservative ambiguity rule as the agent-dir override, using
the leaf env-file module the parent commit extracted so there is no cycle.
@10kH
10kH force-pushed the fix/config-dir-trust branch from 162127f to 2a4c2e8 Compare July 27, 2026 11:30

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head refresh review at 2a4c2e8bbb021961aa917361895b862d489cba84 (rebased past merged #3285, base 69faa20d confirmed ancestor of dev fa2ca8a).

Diff scope verified against prior approved head 162127f: single fix commit 2a4c2e8b extends the existing GJC_CODING_AGENT_DIR project-.env trust-boundary fix to also cover GJC_CONFIG_DIR/PI_CONFIG_DIR via a new trustedConfigDirName() helper in packages/utils/src/dirs.ts, plus matching CHANGELOG and 2 new regression tests in packages/utils/test/agent-dir-trust.test.ts. No unrelated changes.

Trust-boundary re-verification: ran bun test packages/utils/test/agent-dir-trust.test.ts locally against this exact head — 5/5 pass (8 expect calls), including the two new cases (rejects project-.env-planted GJC_CONFIG_DIR; honors shell-inherited GJC_CONFIG_DIR). No regression on the existing agent-dir cases.

CI: all applicable checks terminal SUCCESS (Affected path validation, gjc-state-gates ×4, Local public surfaces, Telegram daemon generation guard); Windows/Python/darwin-arm64 legs SKIPPED as path-gated, not failing.

Verdict: APPROVE.


Reviewed by gaebal-gajae (automated red-team review agent).

@Yeachan-Heo

Copy link
Copy Markdown
Owner

Follow-up at exact head 2a4c2e8bbb021961aa917361895b862d489cba84: the prior APPROVE remains current — no new commits since that review.


[repo owner's gaebal-gajae (clawdbot) 🦞]

@Yeachan-Heo
Yeachan-Heo merged commit f9d4175 into Yeachan-Heo:dev Jul 27, 2026
21 checks passed
10kH added a commit to 10kH/gajae-code that referenced this pull request Jul 27, 2026
resolveConfigPaths (native-skill-hook.ts:156) built its config locations from
raw process.env:

  GJC_CONFIG_DIR ?? PI_CONFIG_DIR ?? ".gjc"
  GJC_CODING_AGENT_DIR ?? join(os.homedir(), configDirName, "agent")

Bun loads cwd/.env into process.env before any module runs, so both values are
repository-controlled. The config.yml these paths select carries
skills.customDirectories — the directories the agent then loads skills from —
so this is the same escalation trustedAgentDirOverride (Yeachan-Heo#3285) and
trustedConfigDirName (Yeachan-Heo#3295) were added to close, reached through a call site
that never went through them.

Measured on a scenario repo whose .env sets GJC_CODING_AGENT_DIR to a planted
directory: getAgentDir() correctly resolved to /home/woody/.gjc/agent, while the hook
loaded the planted config.yml and returned its customDirectories. After the
change the injected entry is gone and a config in the trusted agent dir is still
honored.

getAgentDir()/getConfigDirName() resolve to the same locations this built by
hand — dirs.agentDir is join(os.homedir(), getConfigDirName(), "agent") absent a
trusted override — so only the untrusted case changes.

Regression drives a child process, since the planted value is only visible to a
process started with that cwd. Restoring the raw reads fails the two bypass
cases while the trusted-config case keeps passing.
10kH added a commit to 10kH/gajae-code that referenced this pull request Jul 27, 2026
resolveConfigPaths (native-skill-hook.ts:156) built its config locations from
raw process.env:

  GJC_CONFIG_DIR ?? PI_CONFIG_DIR ?? ".gjc"
  GJC_CODING_AGENT_DIR ?? join(os.homedir(), configDirName, "agent")

Bun loads cwd/.env into process.env before any module runs, so both values are
repository-controlled. The config.yml these paths select carries
skills.customDirectories — the directories the agent then loads skills from —
so this is the same escalation trustedAgentDirOverride (Yeachan-Heo#3285) and
trustedConfigDirName (Yeachan-Heo#3295) were added to close, reached through a call site
that never went through them.

Measured on a scenario repo whose .env sets GJC_CODING_AGENT_DIR to a planted
directory: getAgentDir() correctly resolved to /home/woody/.gjc/agent, while the hook
loaded the planted config.yml and returned its customDirectories. After the
change the injected entry is gone and a config in the trusted agent dir is still
honored.

getAgentDir()/getConfigDirName() resolve to the same locations this built by
hand — dirs.agentDir is join(os.homedir(), getConfigDirName(), "agent") absent a
trusted override — so only the untrusted case changes.

Regression drives a child process, since the planted value is only visible to a
process started with that cwd. Restoring the raw reads fails the two bypass
cases while the trusted-config case keeps passing.
Yeachan-Heo pushed a commit that referenced this pull request Jul 28, 2026
…3324)

resolveConfigPaths (native-skill-hook.ts:156) built its config locations from
raw process.env:

  GJC_CONFIG_DIR ?? PI_CONFIG_DIR ?? ".gjc"
  GJC_CODING_AGENT_DIR ?? join(os.homedir(), configDirName, "agent")

Bun loads cwd/.env into process.env before any module runs, so both values are
repository-controlled. The config.yml these paths select carries
skills.customDirectories — the directories the agent then loads skills from —
so this is the same escalation trustedAgentDirOverride (#3285) and
trustedConfigDirName (#3295) were added to close, reached through a call site
that never went through them.

Measured on a scenario repo whose .env sets GJC_CODING_AGENT_DIR to a planted
directory: getAgentDir() correctly resolved to /home/woody/.gjc/agent, while the hook
loaded the planted config.yml and returned its customDirectories. After the
change the injected entry is gone and a config in the trusted agent dir is still
honored.

getAgentDir()/getConfigDirName() resolve to the same locations this built by
hand — dirs.agentDir is join(os.homedir(), getConfigDirName(), "agent") absent a
trusted override — so only the untrusted case changes.

Regression drives a child process, since the planted value is only visible to a
process started with that cwd. Restoring the raw reads fails the two bypass
cases while the trusted-config case keeps passing.
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.

2 participants