Skip to content

Fix: gitignore the .env that adk create writes so scaffolded agents cannot commit their API key - #364

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/cli-create-gitignore-dotenv
Open

Fix: gitignore the .env that adk create writes so scaffolded agents cannot commit their API key#364
AmaadMartin wants to merge 2 commits into
mainfrom
fix/cli-create-gitignore-dotenv

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    Closes: #issue_number
    Related: #issue_number
  2. Or, if no issue exists, describe the change:
    Problem: adk create writes the user's raw GOOGLE_API_KEY into <agent>/.env (generateEnvFile, dev/src/cli/cli_create.ts) and creates no .gitignore. The scaffolded folder is a brand-new directory in the user's cwd, so the repo's own root .gitignore does not protect it, and the very first git init && git add . && git commit in that folder commits a live credential. A search for gitignore across dev/ returned zero hits before this change.

adk-python already closes this hole in src/google/adk/cli/cli_create.py (_ensure_dotenv_gitignored, called from _generate_files immediately after the .env write). This ports that guarantee.

Solution: generateFiles() now calls ensureDotenvGitignored(agentDir) directly after writing .env, so a .gitignore listing .env always sits next to the secret it protects. A .gitignore that is already present is appended to, never overwritten — its entries are kept verbatim and .env is listed at most once.

Why the append path exists rather than a one-line saveToFile(gitignorePath, '.env\n'): generateAgentFolder() does not guarantee an empty directory. removeFolder() (dev/src/utils/file_utils.ts) catches fs.rm failures and only logs, and createFolder() swallows mkdir failures the same way, so scaffolding proceeds into a folder that survived deletion. I reproduced this against the built CLI (see E2E below): with the parent directory read-only, demo2/.gitignore survived and the unconditional-write version would have destroyed the user's existing entries. The rationale is recorded in the function's doc comment so the branch is not deleted later as dead code.

Design notes:

  • cli_create.ts performs no direct node:fs I/O — all of its file access goes through dev/src/utils/file_utils.ts. That module had no plain-text read (loadFileData is JSON-only and throws on anything else), so this adds one generically named primitive, readTextFile, a 3-line delegation to fs.readFile(path, {encoding: 'utf-8'}). It is an internal util and is not re-exported from dev/src/index.ts.
  • The dedupe check splits on /\r?\n/, not '\n'. Python's str.splitlines() splits on \r\n, so a CRLF-authored .gitignore whose line reads .env\r\n is correctly recognised there; the naive split('\n') yields '.env\r' and would append a duplicate entry on Windows-authored files. This is parity with the reference, and it has its own regression test.
  • ensureDotenvGitignored is module-private in cli_create.ts, mirroring the reference's placement; only the generic readTextFile primitive went into file_utils.ts.
  • No new dependency, no public API change, no change to the contents of .env, agent.{ts,js}, package.json or tsconfig.json. The one user-visible difference is the extra .gitignore in the "Created the following files in ..." listing, which is picked up automatically from listFiles(agentDir).
  • One line of pre-existing code changed: the .env write now uses the same DOTENV_FILE_NAME constant as the ignore entry, so the file written and the entry ignoring it cannot drift apart.

Collision check (required before starting): gh pr list --repo AmaadMartin/adk-js --state open --limit 100 returned no PR adding a .gitignore to scaffolded agents. The two open PRs touching the same files are adjacent only, in disjoint regions: #286 (GOOGLE_GENAI_USE_ENTERPRISE rename inside generateEnvFile, plus .env content assertions) and #313 (assertions in cli_create_test.ts). Neither lands this change and neither conflicts with it, so this branches from main rather than stacking.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

npx vitest run --project unit:dev dev/test/cli/cli_create_test.ts dev/test/utils/file_utils_test.ts
  -> Test Files 2 passed (2) | Tests 34 passed (34)
npm run build   -> exit 0
npm run lint    -> exit 0
npx prettier --check <the four changed files>  -> "All matched files use Prettier code style!"

Seven new cases in dev/test/cli/cli_create_test.ts (all driven through the public createAgent(...) entry point) and two in dev/test/utils/file_utils_test.ts. No existing test was edited, renamed, skipped or deleted; the only changes to existing test scaffolding are additive — two symbols added to the file_utils mock factory and two default mock values in beforeEach.

New-code coverage measured with @vitest/coverage-v8 over dev/src/cli/cli_create.ts and dev/src/utils/file_utils.ts: every added line and every added branch is executed (uncovered lines in both files are all pre-existing code untouched by this PR). Coverage thresholds in vitest.config.ts were not modified.

Proof the tests can fail. Each new test was run against mutated source and observed to fail:

Mutation Result
Delete await ensureDotenvGitignored(agentDir); from generateFiles 5 failed — ...when none exists, ...Vertex AI backend too, both append cases, empty-file case. expected "spy" to be called with arguments: [ StringContaining ".gitignore", …(1) ]
content.split(/\r?\n/) -> content.split('\n') 1 failed — only the CRLF case: expected "spy" to not be called with arguments: [ StringContaining ".gitignore", …(1) ]
Drop the separator computation (always append '\n' + entry) 4 failed — produces 'node_modules\n\n.env\n' / '\n.env\n'
Replace the whole helper with the unconditional saveToFile(gitignorePath, '.env\n') 4 failed — both append cases and both dedupe cases, i.e. the tests do pin the data-loss regression
readTextFile -> .catch(() => '') (swallow) 1 failed — readTextFile throws when readFile rejects
readTextFile -> drop {encoding: 'utf-8'} 1 failed — readTextFile returns the file contents as text

Two pre-existing conditions in this sandbox, unrelated to this PR and identical on main:

  • dev/test/cli/cli_create_test.ts > should handle Vertex AI selection with gcloud defaults fails when GOOGLE_CLOUD_LOCATION/GOOGLE_CLOUD_PROJECT are set in the ambient environment (the suite is not hermetic; already addressed by open PRs Fix: scrub ADK environment variables from unit test runs #302/Fix: scrub ambient DATABASE_URL across the whole dev CLI unit suite #348). Verified failing on main without this change; the runs above unset those two variables.
  • npm run ts:check exits 2 with 280 errors, all in core/test/** and tests/integration/**. The output is byte-identical with and without this branch (diff of both logs is empty), and none of the errors reference the four files changed here.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

All three paths were exercised against the built CLI with no mocks.

  1. Fresh scaffold — the reported bug:
$ mkdir scratch && cd scratch
$ node <adk-js>/dev/dist/esm/cli_entrypoint.js create demo-agent -y --api_key fake-key-not-real
Created the following files in .../scratch/demo-agent:
  - .env
  - .gitignore
  - agent.ts
  - node_modules
  - package-lock.json
  - package.json
  - tsconfig.json

$ od -c demo-agent/.gitignore
0000000   .   e   n   v  \n
0000005

$ cd demo-agent && git init -q . && git add . && git status --short | grep -c '\.env$'
0            # before this change the raw GOOGLE_API_KEY was staged here
  1. Append path — a .gitignore that survived folder creation. Made the parent directory read-only so removeFolder()'s fs.rm fails and is swallowed, exactly as it can in the wild:
$ mkdir demo2 && printf 'node_modules' > demo2/.gitignore && chmod a-w .
$ node <adk-js>/dev/dist/esm/cli_entrypoint.js create demo2 -y --api_key fake-key-not-real
$ od -c demo2/.gitignore
0000000   n   o   d   e   _   m   o   d   u   l   e   s  \n   .   e   n
0000020   v  \n
0000022

The user's node_modules entry is preserved, .env is appended, and no blank line is introduced.

  1. Idempotence — re-ran (2) against the resulting node_modules\n.env\n: the file came back byte-identical, and git add . staged no .env.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

CI note (fork validation run)

All checks pass on the final run: run-tests (ubuntu-latest), run-tests (windows-latest), run-tests (macos-latest), run-tests (cross-language), check-license, auto-assign.

Two pre-existing, unrelated timeout flakes on the non-Linux legs needed re-runs, and both were reproduced on sibling PRs that touch none of these files:

  • macOS — tests/integration/app_loader/app_loader_test.ts > should discover apps vs agents across directories and standalone files, Test timed out in 40000ms. Same failure on feat/js-environment-tools; open PR Fix: attribute cold AgentLoader discovery cost to beforeAll to stop macOS CI flake #260 targets exactly this cold-discovery flake.
  • Windows — core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout, Test timed out in 5000ms.

Both are green on the final run. Ubuntu passed on every attempt, including the first.

Amaad Martin added 2 commits July 30, 2026 22:48
file_utils has no plain-text read: loadFileData is JSON-only and throws on
anything that is not JSON, so a caller that needs the raw contents of a text
file has to reach around the module into node:fs. Add the missing primitive.
adk create writes a raw GOOGLE_API_KEY into <agent>/.env and creates no
.gitignore, so the first 'git add . && git commit' in a freshly scaffolded
agent folder commits a live credential.

Write a .gitignore listing .env alongside the .env itself, matching
_ensure_dotenv_gitignored in adk-python's cli_create.py. An existing
.gitignore is appended to rather than overwritten, because removeFolder()
only logs when deletion fails and scaffolding then proceeds into a folder
whose .gitignore survived; entries are kept verbatim and .env is listed at
most once.
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