Skip to content

Scope the Engine's own git and gh credentials, so a wrong-repo write fails instead of being reported afterwards #679

Closed

Description

@serge-ivo

Follow-up to #676, which shipped the detection half in 7d246958. That change halts a run after a wrong-repo write and names the repository. It does not stop the write. This issue is the half that does.

The problem, measured

The Engine is a claude --dangerously-skip-permissions child process on the owner's machine, spawned at packages/browser-runner/src/coding/headless.ts:420 with env: mergeEnv(process.env, this.config.env) — the machine env inherited wholesale. It therefore reaches GitHub with the account's credentials:

  • git push is over SSH. Both remotes are git@github-personal:… (ProAgentStore/platform and proappstore-online/platform). Git ignores userinfo on an ssh URL, so no token of any kind can gate these pushes.
  • gh is authed from the keyring, account serge-ivo, scopes admin:org, delete_repo, gist, repo, workflow — every org the account can see.
  • Nothing injects a GitHub credential today. grep -rn "GH_TOKEN\|GITHUB_TOKEN" packages/browser-runner/src workers/api/src returns only redaction logic. grep -rni "credential\.helper\|GIT_CONFIG_COUNT\|ASKPASS\|insteadOf\|GIT_SSH_COMMAND" returns 0.
  • The Engine steers its own credential. Observed on two runs: export GH_CONFIG_DIR=~/.config/gh-personal; gh pr merge 111 --merge --repo proappstore-online/platform. Anything env-based is therefore a speed bump, not a boundary, unless the engine is also denied the ability to re-point it.

The tension that has to be designed around, not through

#676 item 2 wants writes scoped; item 3 wants reads broad, and gives a real example. A process holds exactly one gh token, so:

  • scoped GH_TOKEN → gh pr create -R other/repo fails at the credential (good) and gh pr view 138 --repo proappstore-online/platform fails too (breaks the protected pattern);
  • broad read-only GH_TOKEN → reads fine, but the engine cannot open a PR on its own registered repo.

Verified this is not hypothetical: the cross-repo read is the Engine running gh, not a platform tool. Instance a185b1db ("PAGS Coder"), session csess_42bdfe20, 2026-08-16T01:38:04Z, the Pilot's own instruction: Please run: `gh pr view 138 --repo proappstore-online/platform --json tit…` . The platform's github_read_pull was never called on 138 from that instance.

So the answer is a per-invocation credential chooser, not a single injected token.

Sketch

A runner-owned bin directory prepended to the Engine's PATH, holding a gh shim that classifies argv and routes the credential:

Plus, for git: GIT_CONFIG_* env injecting a credential helper, and url.<https>.insteadOf rules to force github SSH remotes through it.

Risks this carries, stated up front

  1. Forcing SSH → https means taking over the machine's git configuration. The owner uses a custom github-personal host alias; rewriting it affects their own reads, not just the agent's. This is the single biggest design risk and may be a reason to scope this issue to gh only and accept that git push stays ungated.
  2. gh's argv surface is large. Misclassifying a write as a read is a hole; default-denying unknown subcommands breaks reads. The classification needs to be a closed, reviewed vocabulary — the same shape as packages/browser-runner/src/coding/repo-write.ts.
  3. A shim on PATH is bypassable by absolute path (/opt/homebrew/bin/gh) or by the engine re-exporting its own env, as it already does. This raises the cost of a wrong-repo write; it does not make one impossible. Anything claiming otherwise in the UI would be worse than nothing.
  4. Engine-agnostic? Only Claude Code reports acts, but a PATH shim applies to every engine — so unlike A coding instance can push to and open PRs on any repo the user can reach, not just its registered one #676's gate, this one would work on Codex/Grok too. That is a point in its favour.

Acceptance

  • A write to an unregistered repo through gh fails at the credential layer, with the refused repo in stderr, and the run reports that rather than a generic failure.
  • A cross-repo gh pr view still succeeds — pinned by a test, because it is the property most likely to be lost.
  • Whatever remains bypassable is stated in coding_diagnostics alongside githubApp.writeScope, replacing the enforcement: "acts-observed-halt" value A coding instance can push to and open PRs on any repo the user can reach, not just its registered one #676 shipped.

Activity

  1. added
    P1: blocks external usersMust be true before someone who is not the owner can run an agent (#68)
    enhancementNew feature or request
    securitySecurity hardening / audit finding
    backendBackend / Worker / API work
    runnerpackages/browser-runner + packages/cli — ships only via an npm publish
    coderThe Coder wedge agent (#68) — Engine, Pilot, Co-pilot, Loop, Overseer
    on Aug 18, 2026
  2. serge-ivo commented on Aug 18, 2026

    @serge-ivo
    ContributorAuthor

    Audit — still LIVE. One citation drifted, and one premise is now better than the issue states.

    Citation correction

    packages/browser-runner/src/coding/headless.ts:420 now reads if (this.oneShot) {. f4efe71b (#700) moved the spawn. The line the issue means is :432, and it says exactly what was claimed:

    		const proc = spawn(this.cmdBin, args, {
    			cwd: this.config.workDir,
    			env: mergeEnv(process.env, this.config.env),

    (There is a second, identical merge at :515, and resolveEngineAuth reads the same expression at :254 — so any credential decision has three sites, not one. Worth knowing before starting.)

    The core claims re-verified today

    • grep -rni "credential\.helper\|GIT_CONFIG_COUNT\|ASKPASS\|insteadOf\|GIT_SSH_COMMAND" packages/browser-runner/src workers/api/src → still 0.
    • git remote -v on this checkout → git@github-personal:ProAgentStore/platform.git, both fetch and push. SSH, so no token can gate it.
    • packages/browser-runner/src/coding/repo.ts:113 states the same fact in code, which is a useful corroboration: "Only https carries a credential: git ignores userinfo on an ssh URL, so injecting there would…"

    Premise update — the token-minting half already exists and is production-wired

    The issue says "Nothing injects a GitHub credential today", citing a grep for GH_TOKEN/GITHUB_TOKEN. That is true of the Engine's process env, and it should be narrowed to that, because the platform does mint and ship a repo-scoped credential on an adjacent path:

    • workers/api/src/lib/git-credentials.ts — "the ONE place a clone credential is minted, for any git provider" — calls repoScopedInstallationToken(env, userId, owner, name) at :107.
    • It reaches the runner as {username, token} (packages/browser-runner/src/coding/runtime.ts:37-39) and is embedded in the clone URL by authenticatedCloneUrl (repo.ts:118), never in the engine's environment.

    Two consequences for this ticket, both favourable:

    1. repoScopedInstallationToken is not a thing to build — it is a thing to route. The sketch's "real gh with a repo-scoped installation token" already has its producer, tested and deployed.
    2. There is a precedent for treating credential destination as a security control, with a written rationale to copy. mayAttachCloneCredential (lib/git-providers.ts:256, doc at :23-29) exists because {githubRepo:"me/private", cloneUrl:"https://attacker.example/x.git"} would otherwise hand a real installation token to an arbitrary host. That is the same shape as "a write verb naming an unregistered repo must not get the credential", one layer up. The gh-shim classifier in the sketch should be argued from it rather than from first principles.

    On risk 1 (rewriting the machine's git config) — a reason to scope this to gh only

    The owner's remotes use a custom github-personal SSH host alias, and repo.ts:113 already documents that ssh userinfo is ignored. Forcing https + insteadOf would therefore mean rewriting configuration the owner uses for their own work, on their own machine, to gate an agent. My read is that the gh-only scope is the right first cut and that git push staying ungated should be stated in coding_diagnostics rather than silently accepted — which is already the issue's third acceptance criterion, and is the one I would not let slip.

    Verified vs inferred

    • Verified: the corrected line and both sibling merge sites; both greps re-run today; git remote -v; git-credentials.ts:107 and its call chain to authenticatedCloneUrl; mayAttachCloneCredential's existence and rationale.
    • Not re-verified: the two live observations in the issue body (the engine re-exporting GH_CONFIG_DIR; the cross-repo gh pr view 138 on session csess_42bdfe20). Those are historical run records; I did not re-read the traces.

    Leaving open, labels unchanged.

  3. serge-ivo commented on Aug 18, 2026

    @serge-ivo
    ContributorAuthor

    Citation corrected before this goes to a dev agent (2026-08-18)

    This issue cites packages/browser-runner/src/coding/headless.ts:420. That line moved — #700's per-tool result rendering shifted it. Today :420 is the one-shot start guard:

    // A one-shot engine has nothing to start until there is a turn to run; starting it here
    // is what produced the instant "exited with code 1".
    if (this.oneShot) {

    The site this issue is actually about is now :432:

    const proc = spawn(this.cmdBin, args, {
        cwd: this.config.workDir,
        env: mergeEnv(process.env, this.config.env),

    mergeEnv(process.env, …) is the inheritance — the Engine gets the machine's whole environment, and with it whatever git and gh credentials that machine holds. The claim is unchanged; only the coordinate rotted.

    Per #707: quote the line, then cite it. The quote above is the durable anchor; treat :432 as a locator that may itself move.

    #676 folded in here

    #676 is closed as folded into this issue. Its detection half shipped; everything it had left was this, in less detail. The asymmetry it recorded must survive and is restated here: the first out-of-scope write still LANDS. coding_diagnostics reports enforcement: "acts-observed-halt" with the note "A write naming a repository outside this list halts the run and is recorded. The first such write still LANDS — the engine uses this machine's own git and gh credentials, which are not scoped." Detection after the fact is not containment, which is the whole reason this issue exists.

  4. serge-ivo commented on Aug 18, 2026

    @serge-ivo
    ContributorAuthor

    The shim shipped — 7e28a7cc + 12aaf484. The credential-broker half is rejected on the code

    CI 32116965397, Deploy API Worker 32116965417 and Publish to npm 32116965667 all green.
    @proagentstore/cli@0.4.56 is on npm — the shim only reaches a machine through that.

    Adopted the shim; rejected the credential routing

    The sketch's "write verb + registered repo → real gh with a repo-scoped installation token" is
    wrong on this codebase, and the audit comment's own finding is what makes it wrong:

    • repoScopedInstallationToken is minted in the cloud and delivered once at session start, for
      the clone. It lives about an hour; a coding session does not. Swapping the machine's working login
      for it turns containment into an outage partway through a long run.
    • It would break the property this ticket says must survive: a cross-repo gh pr view --repo <other org>/<repo> is a different installation, so an installation token cannot read it. The
      machine's own login can.
    • It buys nothing. The harm is the wrong-repo write landing, and refusing to exec stops that
      strictly earlier than handing over a credential that would have failed anyway.

    So the guard refuses; it never brokers.

    What shipped

    packages/browser-runner/src/coding/gh-guard.ts — a generated POSIX-sh shim ahead of the real
    gh on the Engine's PATH. Closed write vocabulary (one TS constant generating the case arms,
    the repo-write.ts shape), --repo/-R/--repo= extraction, and mutating gh api classified off
    the /repos/{owner}/{repo} path. Refusal is exit 3 with the repository in stderr; everything else
    execs through unchanged.

    sh and not Node deliberately: this runs on every gh the Engine invokes and must not depend on
    resolving a module out of a bundle whose layout differs between the monorepo and the published CLI.
    The tests execute the generated script against a fake gh, so what is verified is the artifact
    that ships, not a TypeScript twin of it.

    Scope comes from the cloud, never from the checkout (coding-session-open.ts +
    coding-session-relaunch.ts). The Engine has a shell in that workdir and could git remote set-url
    its way to a wider allowlist. Absent scope means "the platform did not say" and installs nothing —
    never "allow nothing", which would break every session an older cloud starts.

    workers/api/src/lib/coding-write-enforcement.ts replaces the flat
    enforcement: "acts-observed-halt" with a value derived from what the machine reported per
    session. The old sentence survives verbatim for unguarded machines, because there it is still
    exactly true.

    What it does not stop — stated, because a gate that overstates itself is worse than no gate

    Four gaps, reported by the runner and passed through coding_diagnostics verbatim so the two
    cannot drift:

    1. git push is ungated. Remotes are SSH; repo.ts:113 says why in code — "Only https carries
      a credential: git ignores userinfo on an ssh URL"
      . There is no token to withhold.
    2. A PATH shim is bypassable — absolute path, or re-exporting PATH.
    3. gh api graphql is unclassified — a mutation can travel in the query text.
    4. A write with no --repo is not gated. This one I added on review (12aaf484), because it is
      the gap a reader would infer the opposite of. The shim only checks a write that names a repo:
      if [ "$write" = "1" ] && [ -n "$repo" ]; then
      Otherwise gh resolves the target from the working directory. Ordinarily that IS the registered
      repo — refusing it would break gh pr create inside the agent's own checkout, the common case —
      but the Engine can cd. Three gaps were listed and there were four; the count is now asserted so
      a fifth cannot enter the shim without entering the sentence the owner reads.

    No git-config takeover. No insteadOf, no credential.helper, no GIT_SSH_COMMAND — your
    github-personal SSH alias is untouched, which was the issue's own biggest stated risk.

    Verified

    16 invariants watched failing under three separate neutralisations, including the acceptance
    criterion most likely to be lost — the cross-repo gh pr view 138 --repo proappstore-online/platform
    from session csess_42bdfe20 runs as a test and still succeeds. Suite 9,891 passing, all gates
    green. engine-env.ts is an extraction (mergeEnv and its tests move out) forced by the file-size
    ratchet, not a new idea; headless.ts stayed under 959 without raising the pin.

    Live, after the deploy — GET …/coding/diagnostics on a185b1db (PAGS Coder):

    "enforcement": "acts-observed-halt",
    "ghGuard": { "sessionsGuarded": 0, "sessionsTracked": 0, "gaps": [], "reasons": [] }

    The new block is present and honest: no machine is running 0.4.56 yet, so nothing claims to be
    guarded and the old sentence still stands for this machine. That is the designed behaviour, not a
    missing piece.

    What I could not observe, plainly

    Nobody has watched a wrong-repo write actually be refused. That needs a live autonomous run on
    your machine, with the published CLI, against your real gh credentials — which is exactly what
    this issue is about and exactly what I will not experiment with. The green suite is evidence about
    the shim and the wiring; it is not evidence about behaviour anyone watched.

    The check when you next run pags up (≥ 0.4.56) and start a session: coding_diagnostics should
    report ghGuard.sessionsGuarded ≥ 1 and an enforcement value naming the guard. If it reports
    gh-not-found or install-failed, that is the shim telling you it failed open — the Engine
    still works, it is just not gated.

    Leaving open for that observation.

  5. serge-ivo commented on Aug 23, 2026

    @serge-ivo
    ContributorAuthor

    Audit verdict: DELIVERED — the observation this was left open for has now been made. Closing.

    7e28a7cc + 12aaf484 shipped the shim; the closing comment left this open for one thing, in its own words:

    The check when you next run pags up (≥ 0.4.56) and start a session: coding_diagnostics should report ghGuard.sessionsGuarded ≥ 1 and an enforcement value naming the guard.

    Measured live, 2026-08-23

    GET /v1/instances/a185b1db-…/coding/diagnostics (PAGS Coder), runner 0.4.57 on RLs-MacBook-Air.local:

    "enforcement": "gh-guard-partial+acts-observed-halt",
    "ghGuard": {
      "sessionsGuarded": 1,
      "sessionsTracked": 2,
      "gaps": [
        "`git push` is not gated — this machine's remotes are SSH, so there is no credential to withhold.",
        "A PATH shim is bypassable: invoking `/opt/homebrew/bin/gh` by absolute path, or re-exporting PATH, skips it.",
        "`gh api graphql` is not classified — a mutation can travel inside the query text.",
        "A write with no `--repo` is not gated: gh resolves the target from the working directory, so a `cd` into another checkout on this machine reaches it."
      ],
      "reasons": ["the platform sent no repository scope for that session (its repo has no GitHub coordinates)"]
    }

    That is the previous comment's "ghGuard": { "sessionsGuarded": 0, "sessionsTracked": 0, "gaps": [], "reasons": [] } replaced by a real one. The flat enforcement: "acts-observed-halt" #676 shipped is gone, which is AC3.

    The three acceptance criteria

    1. A gh write to an unregistered repo fails, with the repo in stderr. VERIFIED by test, and the test executes the shipped artefact rather than a TypeScript twin — packages/browser-runner/src/coding/gh-guard.test.ts:45-48 runs the generated sh against a fake gh and asserts stderr contains proappstore-online/platform.
    2. A cross-repo gh pr view still succeeds. VERIFIED — gh-guard.test.ts:82-84, the literal csess_42bdfe20 call from the issue body: expect(r.stdout).toContain("REAL GH: pr view 138 --repo proappstore-online/platform").
    3. What remains bypassable is stated in coding_diagnostics alongside githubApp.writeScope. VERIFIED LIVE above; the four gaps are passed through verbatim from the machine (workers/api/src/lib/coding-write-enforcement.ts:67), so the sentence and the guard cannot drift.

    VERIFIED vs INFERRED

    • Verified: the live payload above; both test assertions; that writeEnforcementReport derives enforcement from what the machine reported per session (coding-write-enforcement.ts:60-91).
    • NOT observed, and still not: nobody has watched a wrong-repo write actually be refused in a real run. That is unchanged, and it is deliberately not a reason to keep this open — it is an experiment against the owner's live gh credentials that the implementer refused to run for the right reason, and the check the implementer named as the closing bar has now been met.

    One residual, filed here as a note rather than a new ticket

    sessionsGuarded: 1 of 2. The unguarded session's reason is "the platform sent no repository scope for that session (its repo has no GitHub coordinates)" — ghScope is repo.githubRepo ? [repo.githubRepo] : undefined at workers/api/src/lib/coding-session-open.ts:264 and coding-session-relaunch.ts:52. So a local-path repo with no GitHub coordinates gets no shim at all. That is honest (reasons[] says so) and arguably correct — the platform cannot name a legitimate target for such a repo, and refusing every write would break gh pr create in the agent's own checkout. Worth a decision if local-path repos become common; not worth reopening this.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1: blocks external usersMust be true before someone who is not the owner can run an agent (#68)backendBackend / Worker / API workcoderThe Coder wedge agent (#68) — Engine, Pilot, Co-pilot, Loop, OverseerenhancementNew feature or requestrunnerpackages/browser-runner + packages/cli — ships only via an npm publishsecuritySecurity hardening / audit finding

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions