feat(vmtest-harness): propagate the host GITHUB_TOKEN into the Tart guest for pattern (b) - #4933
Open
mac-duetto wants to merge 3 commits into
Open
feat(vmtest-harness): propagate the host GITHUB_TOKEN into the Tart guest for pattern (b)#4933mac-duetto wants to merge 3 commits into
mac-duetto wants to merge 3 commits into
Conversation
…for pattern (b)
`vmtest run branch` cloned the public repository anonymously inside the guest,
sharing one github.com rate-limit quota with the host and every concurrent guest
on the same egress IP. The host has a GITHUB_TOKEN; the guest had no credential
plumbing at all. It does now, for pattern (b) only.
Two findings shape the implementation and are recorded at the change site so
they are not rediscovered:
- `credential.helper store` silently does nothing here. Git consults a helper
only after a 401, and github.com serves a public repository with 200 — so no
challenge, no helper call, and a deliberately invalid token still clones
successfully. `http.https://github.com/.extraheader` is used instead: it is
sent preemptively on every request.
- the pinned `tahoe-base` image ships a ~/.gitconfig wired to the interactive
Git Credential Manager. Without clearing it first, writing the header fails
with `cannot overwrite multiple values with a single value`, and a rejected
credential makes guest `git ls-remote` hang in the headless VM. Git's
documented empty-value reset runs first and turns that hang into a
sub-second failure — which matters most on the run where the token expired.
Leak discipline. The token is read straight from the host process environment,
never through conf_load/CONF_KEYS (`print_banner` prints every key's value on
every run, --dry-run included). It crosses to the guest on stdin via
`vm_exec_stdin` only — never as a `vm_exec` argument, which becomes host argv —
into a 0600 include file created under `umask 077`. It never enters `repo_url`
(logged verbatim on success, tail'd on failure) nor `$VMTEST_GUEST_ENV`
(prefixed onto every guest command for the run's duration). Only presence and
the pass/fail outcome are logged. `git config --global --add include.path` takes
the PATH, so the secret never enters a command string.
Gating. The pattern test is the first statement of `provision_github_token`, so
every branch that can `die` sits below it: an expired host token cannot fail
`vmtest run local` or `vmtest run released`, neither of which contacts
github.com. A new `propagate_github_token` boolean (default true) is the second
gate, and the fail-safe direction is withhold. An absent GITHUB_TOKEN is not an
error — the run clones anonymously and exits 0 exactly as before.
Validation runs in a new `preflight_config()` before any VM is cloned, so a
typo'd `VMTEST_PROPAGATE_GITHUB_TOKEN=0` is FAIL[10] in a second rather than
after a 30s+ boot, and `--dry-run` catches it too. It is deliberately not folded
into `conf_load()`, which `vmtest clean` shares: a typo'd override must never be
able to break cleanup.
Verification is an in-guest `git ls-remote` against repo_url — an actual network
proof, not a configuration echo.
Known gap carried forward deliberately: `--add include.path` is not idempotent.
Unreachable today because every run gets a fresh guest clone; switching to
`--replace-all` is the right change only if guest reuse is ever introduced.
Static verification: `bash -n` clean on all three modified shell files;
shellcheck 0.11.0 reports 17 findings before and 17 after, byte-identical after
line-number normalisation (zero new); the `tart`-literal invariant grep is
unchanged; `--dry-run` exits 0 for all three patterns; a malformed boolean
exits 10 at preflight with no VM created.
Closes #4924
🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
… keep git's stderr on the config calls Three adversarial-review findings on paths adjacent to #4924's credential plumbing. The credential path itself is unchanged. --keep now says the VM retains the token (MEDIUM). `--keep` is the moment an operator learns the guest survives, and it printed the boot-and-inspect hint with no mention that the guest still holds a credential. The retention caveat lived only in README.md, which nobody reads while watching a run finish — the feature's residual risk was surfaced nowhere at runtime. Teardown now names the include file's guest path, says base64 is encoding and not encryption, gives the `vmtest clean --include-kept` remedy, and says to REVOKE the token if the VM was shared or copied. The gate is a new CRED_PROPAGATED_PATH global, empty until a credential is actually written into the guest. It is set AT THE WRITE, not after the verification proof, and that placement is the point: from the write onward the credential is on the guest's disk whatever happens next, so setting it on the success path only would leave the run that dies at the proof — an expired token, exactly the case an operator reaches for --keep to inspect — preserving a credential silently. It is not named VMTEST_* for the reason RUN_T0_EPOCH and CONF_EFFECTIVE are not, sharpened here: `propagate_github_token` IS a key, so `VMTEST_PROPAGATE_GITHUB_TOKEN` is a real operator-facing override, and an internal global one word-order away from it is a footgun. The pattern-(b) clone-failure message no longer over-claims (LOW). It asserted "the repository is public, so this is not a credentials failure" — unconditionally true until the guest started carrying a preemptive Authorization header. A token revoked in the up-to-300 s between the ls-remote proof and the clone lands exactly there, while the harness told the operator to rule it out. It now says an anonymous clone should have succeeded, that propagation makes a credential failure possible, and that VMTEST_PROPAGATE_GITHUB_TOKEN=false isolates it. The two git config calls keep git's stderr (LOW). Both sent it to /dev/null. The credential.helper reset is precisely the step whose anticipated failure is a specific git diagnostic — `cannot overwrite multiple values with a single value`, exit 5, which this file's own block comment predicts — so the one failure the design foresaw would have died with harness prose and zero evidence. Verification could not have caught it: it exercises only the success path. Both now capture to $VMTEST_TMPDIR/github-auth-config.log and echo it before the die, matching the verification branch. Neither call takes the credential as an argument, so nothing in that output can be the secret. Also corrected "sub-second" to the measured figure: an invalid-token run failed 1.977 s after the credential step began, covering all four guest round-trips. The load-bearing claim — fast, not a 3-minute hang — is unchanged. VERIFIED ON REAL VMs, not by inspection: - `vmtest run branch --keep` (exit 0, 1013 s): the warning fires verbatim, before the inspection hint, naming /Users/admin/.vmtest/github-auth.gitconfig. Booting the kept VM proves every claim in it — the file is there at `-rw-------`, the header is 97 bytes with 0 newlines (unwrapped), exactly one include.path line, and credential.helper reads back as the empty string, so the base image's GCM chain really was cleared. - pattern (c) `--keep` reaching the same teardown branch WITH GITHUB_TOKEN set in the environment: hint printed, 0 warning lines, 0 credential work. The silence is the pattern gate, not an absent token. - bash -n clean; shellcheck 0.11.0 byte-identical to the origin/main baseline at 17 findings (the one new SC2034 is suppressed at source with a directive naming the cross-file consumer, rather than left as a fourth silent warning); --dry-run 0 for all three patterns; malformed boolean still FAIL[10]. - `vmtest clean --include-kept` removed both kept VMs; tart list back to tahoe-base alone. Refs #4924 🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4924
vmtest run branchcloned the repo anonymously, subject to GitHub's anonymousrate limit on an egress IP shared by the host and every concurrent guest. It now
propagates the host's
GITHUB_TOKENinto the guest as a preemptivehttp.https://github.com/.extraheader.The two findings that shaped the design
credential.helper storesilently does nothing here. Git only consults ahelper after a 401, and github.com serves a public repo with 200 — no
challenge, no helper call. The obvious mechanism looks like it works while
delivering zero rate-limit relief.
extraheaderis sent preemptively on everygithub.com request instead.
The pinned
tahoe-baseimage ships a~/.gitconfigwired to Git CredentialManager, which is interactive. Without git's documented empty-value reset
(
git config --global --replace-all credential.helper '') before the headeris wired in, a rejected credential makes guest
git ls-remotehang indefinitelyin a headless VM rather than fail. This branch confirmed the reset at runtime by
booting a
--keepVM and readingcredential.helperback as the empty string.Leak surface
The token crosses to the guest on stdin only, via
vm_exec_stdin— the samechannel the existing
toolchain.tsvwrite uses.vm_exec/vm_exec_rawaredisqualified because their argument becomes
tart exec … /bin/sh -c "<string>"in host argv. It is never a config key (
print_bannerechoes everyCONF_KEYSvalue to stdout on every run, including
--dry-run), never in$VMTEST_GUEST_ENV(composed once and prefixed onto every guest command for therun's full duration), and never in
repo_url(logged verbatim on success,tail -40'd on failure). Only the booleanpropagate_github_tokenis a configkey; the include path is the only thing that enters a command string.
The 0600 include file is created under
umask 077rather thanwrite-then-
chmod, which would leave a window at a looser mode.Gating
The pattern-(b) test is the first executable statement of
provision_github_token, above every branch that candie. Patterns (a) and(c) never contact github.com, so an expired or revoked host token cannot break
vmtest run localorvmtest run released— proven by a real run, not byinspection. A malformed boolean fails at
preflight_configbefore any VM iscloned; that check is deliberately kept out of the shared
conf_load(), so atypo'd override can never break
vmtest clean.An absent
GITHUB_TOKENis not an error — the run proceeds anonymously exactlyas before.
Verification (real VMs, not
--dry-run)git ls-remotesucceeded with the preemptive header; cloned atb98fb77c; 13/13 binaries, 5/5 daemonsFAILof any kind; gate returned before any credential workterminal prompts disabled; the 60s watchdog never fired — git exited on its own, so no hang--keep0600, oneinclude.pathline, 0 newlines in the header,credential.helperempty--keep, token still setLeak audit — 0 hits in every sink, for both the raw token and its base64
Authorizationform: all run logs,$VMTEST_RUNDIR(snapshotted mid-run beforeteardown deletes it), 642 per-second
ps -Awwsamples across the valid-tokenrun, and
git grepover the branch. A positive control returned 2 hits, so thegrep was not vacuous.
bash -nclean.shellcheck0.11.0: 17 findings onorigin/main, 17 after —diff identical, zero new. The harness's
tart-literal invariant is unchanged(the
lib/verify.shhit is pre-existing onorigin/main).Adversarial review verdict: APPROVE, all ten stated invariants confirmed by
static trace, zero CRITICAL/HIGH. Its three findings are applied in
c2bdea60.Operational caveat
--keepleaves the VM on disk instoppedstate with the credential intact.Base64 is encoding, not encryption. The remedy is
vmtest clean --include-kept;if a kept VM has been shared or copied, revoke the token — deleting the VM
afterward does not undo prior exposure. This is now warned at runtime in the
--keepteardown, not only in the README.Known gap, deliberately not fixed
git config --global --add include.pathis not idempotent — re-provisioning thesame guest without teardown would append a duplicate line. Unreachable today
because every run gets a fresh guest clone; verified as exactly one line on the
kept VM. If guest reuse is ever introduced, switch to
--replace-all.Out of scope, surfaced by this work
miserust download; a transient DNSfailure cost one 57s run here and reports
FAIL[40] … DO NOT REPAIR, whichreads identically to real base-image drift.
lib/verify.sh:1385contains the literaltartin a comment, a pre-existingbreach of the harness's own "
tartonly inlib/vm.sh" invariant.lib/provision.shgains the harness's firstshellcheck disabledirective(one line, with its reason), suppressing an
SC2034on a global whose consumerlives in
vmtest. Three globals of the same shape already sit unannotated inthe baseline.
🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools