Skip to content

fix(gardener): harden first-tree repo PR review path #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bingran-you merged 1 commit into from
Apr 24, 2026
Merged

fix(gardener): harden first-tree repo PR review path #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions .github/workflows/first-tree-sync.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,23 @@ jobs:
GIT_ASKPASS="$askpass_script" git -c credential.helper= clone --depth 1 "$tree_repo_url" "$tree_repo_dir"
rm -f "$askpass_script"

- name: Setup pnpm
uses: pnpm/action-setup@v4

- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: "22"
cache: "pnpm"

- name: Install repo dependencies
run: pnpm install --frozen-lockfile
Copy link
Copy Markdown

@yuezengwu yuezengwu Apr 24, 2026

Choose a reason for hiding this comment

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

Running pnpm install --frozen-lockfile here means an unreviewed same-repo PR can execute arbitrary lifecycle/build code before review, with TREE_REPO_TOKEN and GH_TOKEN already present at the job level and CLAUDE_CODE_OAUTH_TOKEN/ANTHROPIC_API_KEY available once node dist/cli.js gardener comment runs. The previous workflow deliberately executed the published first-tree package instead of the PR checkout; switching to pnpm install + pnpm build + node dist/cli.js turns this into a secret-exfiltration path for any contributor who can open an in-repo branch PR. We need to preserve the trusted released binary here, or move this execution to a workflow/event that does not expose secrets to unreviewed PR code.


- name: Build local CLI
run: pnpm build

- name: Install CLIs (first-tree + claude)
# `npx -p first-tree first-tree …` (the gardener-install-workflow
# template's default) is flaky on Node 22 / newer npm — it
# fails with `first-tree: not found` after install. Install both
# globally instead.
#
# Gardener's verdict classifier is designed to shell out to the
# `claude` CLI (see first-tree sync.ts). Pre-install
# @anthropic-ai/claude-code so any future upstream CLI upgrade
# that wires a real classifier into `gardener comment` will
# authenticate via CLAUDE_CODE_OAUTH_TOKEN below without another
# workflow change.
run: npm install -g first-tree @anthropic-ai/claude-code
- name: Install Claude Code
run: npm install -g @anthropic-ai/claude-code

- name: Run first-tree gardener
env:
Expand All @@ -90,7 +89,7 @@ jobs:
# bot identity is enough for the self-loop guard.
GARDENER_USER: github-actions[bot]
run: |
first-tree gardener comment \
node dist/cli.js gardener comment \
--pr ${{ github.event.pull_request.number }} \
--repo ${{ github.repository }} \
--tree-path .first-tree-cache/tree \
Expand Down
5 changes: 5 additions & 0 deletions src/products/gardener/engine/classifiers/claude-cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ function runClaude(
"text",
"--model",
model,
"--disable-slash-commands",
"--setting-sources",
"user",
"--tools",
"",
], {
stdio: ["pipe", "pipe", "pipe"],
env,
Expand Down
13 changes: 10 additions & 3 deletions tests/gardener/gardener-claude-cli-classifier.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe("createClaudeCliClassifier", () => {
});

it("scrubs ANTHROPIC_API_KEY from injected env", async () => {
const calls: Array<{ env?: NodeJS.ProcessEnv }> = [];
const calls: Array<{ env?: NodeJS.ProcessEnv; args?: string[] }> = [];
const classifier = createClaudeCliClassifier({
env: {
PATH: "/usr/bin:/bin",
Expand All @@ -157,10 +157,10 @@ describe("createClaudeCliClassifier", () => {
},
spawnImpl: ((
_command: string,
_args: string[],
args: string[],
options?: { env?: NodeJS.ProcessEnv },
) => {
calls.push(options ?? {});
calls.push({ ...(options ?? {}), args });
return makeFakeChild({
stdout: JSON.stringify({
verdict: "ALIGNED",
Expand All @@ -176,6 +176,13 @@ describe("createClaudeCliClassifier", () => {
expect(calls).toHaveLength(1);
expect(calls[0]?.env?.ANTHROPIC_API_KEY).toBeUndefined();
expect(calls[0]?.env?.GARDENER_DIR).toBe("/tmp/gardener");
expect(calls[0]?.args).toEqual(expect.arrayContaining([
"--disable-slash-commands",
"--setting-sources",
"user",
"--tools",
"",
]));
});

it("ClaudeCliClassifierError carries kind and stderr", () => {
Expand Down
Loading