feat(grok): install the Grok CLI at image build time - #16
Conversation
The CLI was being installed at container start by piping x.ai's installer into bash, so every container re-downloaded a 166 MB binary, ran whatever build was newest at that moment, and lost it again on restart. The feature runs the same upstream installer — its platform detection and channel lookup are worth keeping — but does not let it place the result. Two things about its layout make a plain run unusable in an image: * It downloads into a hardcoded "$HOME/.grok/downloads" and only symlinks "$BIN_DIR/grok" at it, so a root install leaves the real binary under /root, unreadable to the remote user. Setting GROK_BIN_DIR moves the symlink and not the binary. Staging a HOME keeps both halves together. * When its bin directory is not on PATH it symlinks grok AND agent into the first writable PATH entry — /usr/local/bin — aimed at the staging directory, which dangles once staging is removed. Putting the staged bin on PATH takes that branch away, and a cleanup pass covers it regressing. The binary is then copied to /usr/local/share/grok/bin/grok and symlinked into /usr/local/bin, matching the shared-prefix shape the other features use. No credential is read at build time, so none can reach an image layer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DYA9iEvPjTgcJCcbXaw6Eh
|
|
||
| staged_bin="${staging}/.grok/bin" | ||
|
|
||
| curl -fsSL "${INSTALLER_URL}" -o "${staging}/install.sh" |
There was a problem hiding this comment.
Unverified installer runs as root
Every feature installation executes the current response from https://x.ai/cli/install.sh without verifying a checksum or signature, so a compromised endpoint can modify arbitrary image contents or install a backdoored CLI. How this was verified: The downloaded file is passed directly to bash without any integrity verification.
There was a problem hiding this comment.
Accurate as a description, but not something this PR can fix, and it points the wrong way on risk.
There is nothing published to verify against. I checked before answering: …/cli/grok-1.0.4-linux-x86_64.sha256 and …/cli/SHA256SUMS both 404 on the primary and the GCS fallback, and the installer performs no integrity check of its own (zero matches for sha256/checksum/gpg/signature in its source). Pinning a hash of the installer script is also not available: it is served dynamically — the same URL resolves the channel's newest build at request time — so a pinned digest would break on every upstream release, and there is no versioned URL for it.
Trust model matches the repo's precedent. src/buildbuddy/install.sh is curl -fsSL https://install.buildbuddy.io | bash. The anchor in both cases is TLS to the vendor's own documented distribution channel.
This PR reduces the exposure it is being flagged for. The code it replaces runs that identical unverified script on every container start, on machines executing customer code. Moving it into the image means it runs once, at build time, and what ships is then fixed by the image digest — reviewable and reproducible even though the fetch itself is unverified. Pinning version narrows it further, to one specific published build rather than whatever is newest at boot.
Worth revisiting if x.ai starts publishing checksums — the feature already knows the exact version it asked for, so verifying the binary would be a small addition at that point.
Generated by Claude Code
There was a problem hiding this comment.
You're right on all three counts, and I shouldn't have flagged this as a meaningful risk without checking whether the vendor actually publishes anything to verify against.
The argument holds: no published checksums means there's nothing to pin, a hash of the installer itself would break on every release since the URL is dynamic, and the PR objectively reduces the attack surface by moving the unverified fetch from every container start to a single build-time event whose result is then fixed by the image digest.
The buildbuddy precedent also applies — the repo already accepts TLS-to-vendor-channel as sufficient trust for installer scripts.
I'll withdraw the concern. If x.ai adds checksum publishing in the future, the feature is already structured to make verification a small addition.
Shall I remember this as a rule: "do not flag curl | bash installer patterns as unverified when the vendor publishes no checksum or signature infrastructure — note it at most as a future hardening opportunity if the vendor adds one"?
| "features": { | ||
| "ghcr.io/devcontainers/features/common-utils:2": {}, | ||
| "grok": { | ||
| "version": "latest" |
There was a problem hiding this comment.
Pinned-version branch lacks coverage
The sole scenario selects latest, while exact versions use a separate positional-argument branch in install.sh; without a pinned scenario that verifies the installed version, CI will not detect an upstream contract change that breaks pinning or installs the wrong release.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Taken in 193a69d — added an ubuntu_pinned scenario on 1.0.4 plus test/grok/ubuntu_pinned.sh asserting grok --version reports it, which is the branch that passes the version as a positional argument.
I'd left it out to avoid a test that breaks when a pinned build is eventually withdrawn, but you're right that the branch is otherwise uncovered, and that failure mode is loud and one line to fix. Noted in the test file so the next person knows to bump both together.
Generated by Claude Code
| HOME="${staging}" PATH="${staged_bin}:${PATH}" bash "${staging}/install.sh" "${VERSION}" | ||
| fi | ||
|
|
||
| binary="$(readlink -f "${staged_bin}/grok")" |
There was a problem hiding this comment.
Readlink bypasses installer diagnostic
When the upstream installer omits grok or leaves a dangling link, readlink -f exits nonzero and set -e terminates the script before the explicit missing-binary check, replacing the actionable diagnostic with an opaque command failure.
| binary="$(readlink -f "${staged_bin}/grok")" | |
| binary="$(readlink -f "${staged_bin}/grok" || true)" |
There was a problem hiding this comment.
Taken in 193a69d. Confirmed the mechanism against coreutils 9.4: readlink -f exits 1 only when a parent component is missing — it returns 0 for a missing final component (printing the path) and 0 for a dangling symlink (printing the target). So the two cases named here are actually caught by the test -x below; the one that isn't is a missing ${staged_bin} itself, where set -e would kill the script with no diagnostic. Practically unreachable, since a failing installer aborts at the run_installer call first — but the guard costs nothing and makes the check independent of upstream's ordering.
Generated by Claude Code
A feature inherits the environment it is installed into, and the invocation
overrode only HOME and PATH. An exported GROK_DEPLOYMENT_KEY therefore made
the installer authenticate and POST that key as a bearer token to
"${GROK_PROXY_URL}/deployment/config" — a credential sent to whatever
endpoint the build environment named, contradicting this feature's own
documented promise that nothing is authenticated at build time.
Reproduced with the key set and the proxy pointed at 127.0.0.1: the
installer logs "Auth: using deployment key" and "Fetching deployment
config". Run through "env -u" for GROK_DEPLOYMENT_KEY, GROK_PROXY_URL,
GROK_CHANNEL, and GROK_BIN_DIR, the same environment installs the public
stable build with no auth and no attempt on the proxy. The latter two matter
for correctness as well: CHANNEL would silently swap the build, and BIN_DIR
would move the bin directory out of the staging tree.
Also guard the readlink with "|| true" so a missing parent component reports
the explicit missing-binary error instead of dying to set -e, and add a
pinned-version scenario covering the positional-argument branch that an
exact version takes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DYA9iEvPjTgcJCcbXaw6Eh
|
All four findings from the first pass are addressed at
CI is green on the new head, and its Generated by Claude Code |
There was a problem hiding this comment.
This review was skipped because it would exceed your organization's monthly flex usage limit. Raise the limit in billing settings or wait until the next billing period resets limits.
Every other install script in the repo carries no prose comments at all — only the odd shellcheck directive — and explains itself in NOTES.md instead. This one had grown 23 comment lines around 50 of code. The code is unchanged. The two things those comments explained that NOTES.md did not already cover, the PATH suppression and the leftover-link cleanup, move into its "Where it installs" section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DYA9iEvPjTgcJCcbXaw6Eh
Same overrun as install.sh, in the tests: the other features' scripts carry shellcheck directives and at most a single-line note where a check's purpose or ordering is not obvious. Both of these had a three-line block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DYA9iEvPjTgcJCcbXaw6Eh
Softlight Overview
UX Score: 5/5
Softlight reviewed this change and found no issues.
To run Softlight again, click here or comment @softlight.
Adds a
grokfeature, so the Grok CLI can be baked into an image the way #15 did fortypescript-language-server.Today
orianna's sandbox runner pipes x.ai's installer into bash on every container start (LocalRunner.create). That means each container re-downloads a 166 MB binary on its boot path, runs whatever build happened to be newest at that moment, and loses it on restart — the CLI lands in$HOME/.grok/bin, which in that deployment is a per-sandbox volume.What the feature does
Runs the same upstream installer — its platform detection, channel lookup, and post-download smoke test are worth keeping — but does not let it place the result. Two things about its layout make a plain run unusable in an image, both found by running it:
$HOME/.grok/downloadsand only symlinks$BIN_DIR/grokat it. A root install therefore leaves the real binary under/root, reachable while building and unreadable to the remote user afterwards. SettingGROK_BIN_DIRdoes not fix it — that moves the symlink and leaves the binary behind it. The feature stages aHOMEso both halves land together, then copies the resolved binary out.PATH, it symlinksgrokandagentinto the first writablePATHentry —/usr/local/binduring a build — pointing at the staging directory, which dangles the moment staging is removed. Putting the staged bin onPATHtakes that branch away; a cleanup pass covers it regressing upstream.The binary is then copied to
/usr/local/share/grok/bin/grokand symlinked into/usr/local/bin/grok, the same root-owned shared-prefix shapeplaywrightandtypescript-language-serveruse. Unlike those two it needs no node feature — the CLI is a single static binary — soinstallsAfteris justcommon-utils.Notes
GROK_DEPLOYMENT_KEYwould make the installer authenticate and POST that key as a bearer token to${GROK_PROXY_URL:-…}/deployment/config. The installer therefore runs withGROK_DEPLOYMENT_KEY,GROK_PROXY_URL,GROK_CHANNEL, andGROK_BIN_DIRcleared, over a stagedHOMEwith no.grok/auth.json. The last two matter for correctness too:CHANNELwould silently swap the build,BIN_DIRwould move the bin directory out of the staging tree.grokis linked. Upstream also links the binary asagent; that is too generic a name to claim in a shared image, and nothing consuming the CLI needs it.GROK_CHANNELis not exposed as an option. The enterprise channel needs a deployment key a feature cannot carry, andversioncovers the reason to want a non-stable channel.versionaccepts an exactX.Y.Z(the installer rejects anything else). The defaultlateststill resolves at build time, so it is fixed for the life of the image either way.Testing
CI's
devcontainer features testran both scenarios in real containers and passed — the TEST REPORT lists six passing scenarios, fiveubuntu(one per feature) plusubuntu_pinned, which only this feature defines.Verified directly while developing, before CI had run:
install.shexecuted end to end. The first run is what surfaced the dangling-agentbug: it loggedSymlinked /usr/local/bin/agent -> /tmp/tmp.HEdZ…/.grok/bin/agent, which broke as soon as staging was cleaned. After thePATHfix those lines are gone.GROK_DEPLOYMENT_KEYset andGROK_PROXY_URLpointed at127.0.0.1:1, the pre-fix invocation logsAuth: using deployment keyandFetching deployment config. Withenv -uapplied, the same environment yields no auth, no proxy attempt, no hijacked bin directory, and the public stable build./usr/local/bin/grok→/usr/local/share/grok/bin/grok, a real root-owned file, mode0755; noagentlink; no staging directory left; nothing written under/root(the empty/root/.grokthat appears later is created by running the CLI, not by installing it).grok --versionruns as a non-root user and reportsgrok 1.0.4, with all five checks intest/grok/ubuntu.shpassing. Run as root the read-only check fails, as it must —test -wis always true for root — which is why the scenarios use"user": "vscode".