Skip to content

Install frontend deps on agent session start#623

Open
mariusvniekerk wants to merge 6 commits into
mainfrom
chore/session-start-frontend-deps
Open

Install frontend deps on agent session start#623
mariusvniekerk wants to merge 6 commits into
mainfrom
chore/session-start-frontend-deps

Conversation

@mariusvniekerk

Copy link
Copy Markdown
Collaborator

Agent sessions can fail immediately on frontend checks when node_modules is missing. This adds a shared session-start hook for Codex and Claude that runs the existing Bun workspace dependency target before work begins.

This keeps both agent entrypoints aligned with the repo's Bun-only frontend tooling and avoids each workflow rediscovering missing Vite+ deps later.

generated by a clanker

mariusvniekerk and others added 2 commits June 30, 2026 15:57
Agent sessions often need Vite+ and the workspace frontend packages before they can run checks or dev commands. Installing the Bun workspace dependencies at session start makes Codex and Claude worktrees start from the expected local tool state instead of failing later on missing node_modules.

Validation: parsed both hook JSON files, checked the shell script with bash -n, and ran the session-start script successfully with Bun.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
The session-start hook runs from a middleman worktree, matching the existing Stop hook convention. Keeping the command form consistent makes the hook configuration easier to read while the shared script remains responsible for running from the repository root.

Validation: parsed both hook JSON files and checked the shared shell script with bash -n.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (d879ba7)

High severity: auto-running worktree-controlled code on session start

  • .codex/hooks.json:8
  • .claude/settings.json:8
  • scripts/hooks/frontend-deps-session-start.sh:7

The new SessionStart hooks automatically execute a script from the checked-out worktree, which then runs make frontend-deps. Because both the script and Makefile are branch-controlled, a malicious PR can modify them, package manifests, lockfiles, or install scripts so that merely starting a Codex/Claude session executes attacker-controlled commands under the reviewer/agent user. This can expose local repository contents and credentials available to the agent process before the reviewer explicitly chooses to run project code.

Remediation: do not auto-run worktree-controlled commands on SessionStart. Require explicit approval for dependency installation in untrusted branches, or call a trusted out-of-tree wrapper that performs only non-script dependency resolution, without routing through the repository Makefile.

Medium severity: dependency hook assumes bun is available

  • scripts/hooks/frontend-deps-session-start.sh:7

The hook unconditionally runs make frontend-deps, which invokes bun install. The checked-in Codex node-deps environment installs dependencies with vp install --frozen-lockfile, and some documented environments may already have dependencies present while bun is not on PATH. In those sessions, the hook can fail even though frontend dependencies are already installed.

Remediation: accept an existing node_modules/vite-plus/bin/vp, or install via vp install --frozen-lockfile with a Bun fallback only when installation is actually needed.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 1m53s), codex_security (codex/security, done, 1m26s) | Total: 3m29s

Agent session startup should not rerun package installation when the local Vite+ binary is already present. When dependencies are missing, using Vite+'s frozen-lockfile installer keeps the session path aligned with the node-deps environment while preserving a Bun fallback for hosts without a global vp command.

Validation: checked script syntax with bash -n; ran the existing-local-vp fast path; exercised missing-local-vp with a stubbed vp and verified it calls `vp install --frozen-lockfile`; exercised the no-vp fallback with a stubbed bun and verified it calls `bun install --frozen-lockfile`.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (ba73755)

High-risk security issue found.

High

  • .codex/hooks.json:8, .claude/settings.json:8, scripts/hooks/frontend-deps-session-start.sh:12, scripts/hooks/frontend-deps-session-start.sh:15
    The new SessionStart hooks automatically run checkout-controlled dependency installation when an agent session starts. In a fresh worktree, the hook executes vp install --frozen-lockfile or bun install --frozen-lockfile; a remote PR author could modify repo-controlled install inputs such as package.json lifecycle scripts and get code executed with reviewer/agent privileges. --frozen-lockfile does not prevent adding or changing root lifecycle scripts.

    Suggested remediation: avoid automatic dependency installation from repo-controlled session hooks for untrusted branches. Move bootstrap logic to trusted tooling outside the checkout or require explicit user action. If automatic install is unavoidable, make the hook immutable from the reviewed branch and disable lifecycle scripts, such as Bun’s --ignore-scripts and equivalent behavior for vp install.


Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 1m44s), codex_security (codex/security, done, 2m10s) | Total: 4m3s

mariusvniekerk and others added 3 commits June 30, 2026 16:21
The session-start setup still needs to prepare frontend tooling automatically, but routing through a branch-controlled helper script and package lifecycle hooks gives untrusted PRs more execution surface than needed. Keep the automatic install while limiting it to the hook definition itself and package-manager installs with lifecycle scripts disabled.

Validation: parsed both hook JSON files; ran the existing-node_modules fast path from the hook command; exercised missing-node_modules with stubbed vp and verified `vp install --frozen-lockfile --ignore-scripts`; exercised the fallback path with stubbed bun and verified `bun install --frozen-lockfile --ignore-scripts`.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
The frontend dependency bootstrap hook is intentionally automatic, but future edits need to preserve the trust boundary: avoid branch-controlled helper scripts or Make targets and disable package lifecycle scripts when installing. Recording that convention keeps the review decision from living only in this PR conversation.

Validation: ran git diff --check.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
Installing with lifecycle scripts disabled is safer but needs a cheap readiness check so SessionStart does not report success without the local Vite+ entrypoint. The project guidance now also states the real trust boundary: this reduces execution surface, but changed hook config still depends on the runner's hook-trust model.

Validation: parsed both hook JSON files; ran the existing-node_modules fast path; exercised stubbed vp and Bun install paths and verified they include --ignore-scripts and satisfy the post-install Vite+ entrypoint check; ran git diff --check.

Generated with Codex (GPT-5)

Co-authored-by: Codex <codex@openai.com>
@roborev-ci

roborev-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (8b00067)

High-level verdict: changes need revision before merge due to one high-risk hook bootstrap issue and one medium dependency-pinning issue.

High

  • .claude/settings.json:8, .codex/hooks.json:8
    The new SessionStart hook automatically runs dependency installation inside the checked-out worktree. A PR author controls package-manager inputs such as package.json, bun.lock, and registry config, while the hook runs with the agent/reviewer environment and network access before an explicit trust decision. --ignore-scripts blocks lifecycle scripts, but registry/auth configuration may still be honored, allowing ambient credentials such as NPM_TOKEN or NODE_AUTH_TOKEN to be sent to an attacker-controlled registry.

    Suggested fix: Avoid auto-installing dependencies for untrusted worktrees, or run the bootstrap in a secretless/network-restricted environment with a fixed trusted registry and ignored project registry config. At minimum, clear package auth environment variables before invoking the package manager and prevent branch-controlled registry config from influencing installs.

Medium

  • .claude/settings.json:8, .codex/hooks.json:8
    When local node_modules/vite-plus/bin/vp is absent, the hook falls back to whichever global vp appears first on PATH. That binary is not pinned by package.json or bun.lock, so it may be stale, incompatible, or untrusted.

    Suggested fix: Remove the global vp fallback and use the repo’s declared package-manager path, such as running bun install --frozen-lockfile --ignore-scripts whenever the local Vite+ entrypoint is missing.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 1m39s), codex_security (codex/security, done, 3m35s) | Total: 5m24s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant