fix(git): the fleet PAT stops being readable inside every agent container (abilityai/trinity-enterprise#615) - #2757
Merged
Merged
Conversation
AndriiPasternak31
marked this pull request as draft
September 14, 2026 00:50
|
Resolve by running |
|
Resolve by merging |
Trinity persists every agent's remote as `<scheme>://oauth2:<PAT>@<host>/<org>/<repo>.git`, which puts the platform GitHub token in `.git/config` on the workspace volume and — because git expands the stored URL into `git-remote-https`'s argv — in the container's process table on every fetch and push. From argv it reaches `ps`, the orphan sweep's reaped-cmdline logging, Vector, the host log files and the logs API, i.e. another agent's LLM context. Git speaks the credential protocol to a helper over stdin/stdout, which never reaches argv and is never persisted. This is the helper plus its two string builders; the producers still build token URLs and are converted next. Three things are load-bearing and each is proven, not asserted (git 2.50.1): - Registered as `trinity`, NOT the filename. Git prepends `git-credential-` to any helper value that is not an absolute path, so registering `git-credential-trinity` resolves to `git-credential-git-credential-trinity`: `git: 'credential-git-credential-trinity' is not a git command`. The helper would never run, and with token-free URLs that is a silent fleet-wide fetch/push outage. - Registered UNSCOPED, host-checked inside the helper. TRINITY_GIT_BASE_URL is a runtime value, so a `credential.<base>.helper` baked at image-build time could only ever name github.com and a self-hosted install would get no helper. Resolving the origin at request time also writes nothing to `~/.gitconfig`, which sits in the agent's repo root and is not ignored. - `.env` FIRST, baked env second — the inverse of startup.sh, deliberately. startup.sh runs at boot where baked env is the freshly-recreated truth; the helper runs in steady state where a rotation (#1967) or a per-agent PAT set after creation (#1264) has live-injected the new token into `.env` while Config.Env is immutable without a recreate. Baked-env-first would authenticate with the revoked token forever. The sweep that reaches existing containers lands with the remediation commit. It probes the helper by EXIT CODE only: `git credential fill` prints the credential on stdout, and this output crosses the docker exec boundary into the platform log. Not closed: an agent reading its own credential. Root ownership is integrity, not confidentiality. That is trinity-enterprise#558 (AAuth). Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four producers built `<scheme>://<userinfo>@<host>/<repo>.git`, not the two the first pass found. All four are converted or deleted, so there is now exactly one remote-URL builder in the product and it carries no credential. 1. `git_service._git_remote_url` — DELETED. Its three call sites (`update_remote_pat`, `rebind_origin_and_push`, `initialize_git_in_container`) use `_credentialless_remote_url`. 2. `startup.sh` CLONE_URL — one form, credential-less. The clone, the fetch and the push authenticate through the `trinity` credential helper. 3. `template_service.clone_github_repo` — DELETED. It passed a token URL as argv to `subprocess.run` on the BACKEND HOST: same bug class, one layer out. No production callers (an export and two test mocks). 4. `skill_service._authenticated_url` — split into `_normalized_url` (no credential) and `_auth_pat_for` (the host decision, unchanged from PR #1901's parse-based check), with the PAT carried in the git child's environment. This one was LIVE: git wrote the spliced URL to `origin`, so the platform PAT sat at rest in `/data/skills-library/*/.git/config` — on the `~/trinity-data` host bind mount, and therefore in every backup and snapshot of it. Ordering is the whole design: **no path strips a credential it has not already replaced.** - `update_remote_pat` writes the token to `.env` over `docker exec` (which works while the agent server is wedged, restarting or OOM — the HTTP write it belts does not), runs the sweep, and only re-points `origin` once a credential provably resolves. - `initialize_git_in_container` SEEDS the credential before anything writes a remote, which is what stops it creating the orphan class it was named for: it used to push with the resolved platform PAT while baking no git env, persisting no row and writing no `.env`, leaving the agent's only credential inside its own origin URL. - `startup.sh`'s origin rewrite becomes conditional. Unconditional was safe only while the replacement URL also carried a token; for an orphan agent it would be destruction of the last credential, at container start, before any backend sweep could harvest it. - `configure_push_remote`'s gate widens from `GITHUB_PAT` to "a credential resolves" — narrowly: the helper reads `.env`, baked env, and the harvest file, which only exists for an agent whose own URL already carried a push credential. An ent#123 tokenless agent resolves nothing and stays blackholed. The harvest deliberately does NOT write `GITHUB_PAT`, which startup.sh exports as GH_TOKEN/GITHUB_TOKEN — that would be a grant. Also: `_agent_has_write_credentials`'s premise ("the global tier is deliberately excluded — a global PAT never reaches a tokenless container's remote") is what this change falsifies, so it is no longer the whole predicate. `_agent_can_push` consults it first and the helper's own ladder second, by exit code, only for agents that already look tokenless. The ent#109 rebind push stays in-container — the history lives only on the workspace volume — but runs as ROOT with the user's PAT in the exec environment, so `/proc/<pid>/environ` is unreadable by the `developer`-uid agent for the 120s push window. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three reachers, no new recurring service: - `sweep_fleet_git_remote_tokens` — a boot-time one-shot. After this change no code path produces a token-bearing remote, so there is no recurring producer for a recurring loop to chase. This is also the only reacher that covers a `restart: unless-stopped` container the Docker daemon brings back after a HOST REBOOT, which never passes through `start_agent_internal`. Fail-open lease, and why that is right here is stated at the function: a duplicate pass of an idempotent, refuse-rather-than-destroy sweep is cheap; never remediating an install with no Redis is not. - `spawn_git_remote_token_scrub` at `start_agent_internal` — the #2069 fire-and-forget shape, deliberately WITHOUT its `auto_sync_enabled` gate. Whether an agent auto-syncs has nothing to do with whether its `.git/config` holds a token, and copying that gate would silently skip every agent that does not. - `startup.sh`'s conditional per-restart rewrite (previous commit). A refusal is queued, not just logged: one operator-queue alert per agent per UTC day, so an operator learns that an agent still holds an embedded credential that Trinity would not remove without a replacement. `PROTECTED_KEYS` gains `GIT_CONFIG_KEY_*` / `GIT_CONFIG_VALUE_*` (by prefix — an unbounded family), `GIT_CONFIG_NOSYSTEM`, `GIT_ASKPASS` and `GIT_PROXY_COMMAND`. `GIT_CONFIG_COUNT` was already listed but its SLOTS were not, so the guard covered the count and left the payload writable — and a slot can set `core.sshCommand`, `core.pager`, `diff.external` or `credential.helper`. `NOSYSTEM` becomes load-bearing here: `/etc/gitconfig` is now where the helper is registered, alongside the #1595 gc guards, so one agent-written `.env` line would disable both. Tests: `tests/unit/test_ent615_token_free_remotes.py`, 66 cases. The helper and the sweep are SHELL, so these EXECUTE them — a Python assertion on script text passes against a script that cannot run, which is exactly the CRIT-1 failure. Mutation-checked: reverting the helper name, the `.env`-first ladder, the exact-host check, the harvest-before-strip order, the root exec, or the credential-less re-point each turns tests red. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…emotes Trinity Rule #1 works backwards here — the code landed first because the mechanism had to be proven by execution before it could be described honestly (the helper NAME and the resolution ORDER were both wrong in the first design and only a real git told us). - Requirements: `github.md` §11.16 (the full requirement, six FRs), plus FR-4 and FR-5 corrected in place. FR-5's parenthetical — "never the global tier, which cannot reach a tokenless container" — is a sentence this change makes FALSE, so it is rewritten rather than left standing. `security.md` §20.9b (the standing rule, the chain it closes, and what it does NOT close). `credentials.md` §3.8 (the ladder as the git credential source of record — `.env` first, and why rung 3 is neither `.env` nor the DB row). - Architecture: `architecture/security.md` Container Security (the helper, the three properties that each cost an iteration, and the honest "root ownership is integrity, not confidentiality" line); `agent-lifecycle.md` (the rebind push's root exec, and why it cannot move to the backend host); `agent-runtime.md` (the `PROTECTED_KEYS` prefix rule). Core `architecture.md` is unchanged — no Architecture-Map path moved. - Feature flows: `github-sync.md` gains a Credential-free remotes section and four corrected passages (the unconditional restart rewrite is now conditional; the write-credentials predicate has a second tier); `github-repo-initialization.md`'s accepted-risk block — "PAT is visible in git remote URL inside container" — is retired, and this change is its changelog; `credential-injection.md` records `.env` as the first rung and why the harvest is not written there. - `docs/migrations/GIT_REMOTE_TOKEN_SCRUB_2026-09.md`: the operator runbook — what the sweep does, what `harvested` / `refused` / `gitmodules_hits` mean, the `.gitmodules` case it cannot fix, and the standing advice to rotate the platform token after adoption (mandatory rather than advisory if any agent reports a `.gitmodules` hit). - `tests/registry.json`: both new files, and the #1967 entry's own "`.env` is not where git authenticates from" premise corrected — that is the sentence this issue inverts. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`/sync-feature-flows` over the branch diff. Three carried claims this change makes false, which is the class the sweep exists to catch: - `template-processing.md` printed the PAT-bearing `CLONE_URL` and the old PAT-gated clone condition (ent#123 had already retired the gate). - `github-sync.md`'s branch-support table pointed at `template_service.clone_github_repo()`, deleted here — it put a token URL on the backend host's **argv** and had no production callers. - `skills-library-sync.md` printed the splice itself. That one was live: git wrote the spliced URL to `origin`, so the platform PAT was at rest in `/data/skills-library/*/.git/config` — on the `~/trinity-data` host bind mount, and so in every backup of it. The host DECISION it documents is unchanged and still parse-based (PR #1901): an `http.extraHeader` goes to whatever host git connects to, so "is this host ours" still gates whether the credential travels. Its status allow-list and the scrubbers stay exactly as they are — a stored source row can still carry userinfo of its own. Two gained a seam worth naming: - `async-docker-operations.md`: `execute_command_in_container` now takes `environment` (Exec Create body, **not argv** — argv is the leak) and `user` (root, so `/proc/<pid>/environ` is unreadable by the `developer`-uid agent). Also records the pre-existing trap that its `timeout` is accepted and forwarded nowhere, and why a caller needs BOTH bounds. - `agent-lifecycle.md`: the two fire-and-forget start hooks, and why the ent#615 one is deliberately not behind #2069's `auto_sync_enabled` gate. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Not cosmetic. Each of these states, as a fact a later reader will rely on, something that was true before ent#615 and is false after it — which is the class of stale comment that makes the next change wrong. - `github_pat_propagation_service`'s module docstring said "`.env` is not where git authenticates from", with the reasoning that made #1967's fix correct. That premise is exactly what this issue inverts, and deliberately: the helper reads `.env` FIRST, because a rotation does not recreate the container, so `Config.Env` keeps the revoked token until the next recreate. The old text is kept in past tense with the inversion stated under it, rather than deleted — it is why the current shape is what it is. - `_apply_pat_to_agent`'s docstring now says why the HTTP `.env` write stays primary even though `update_remote_pat` writes the same line over `docker exec`: only the HTTP path runs `sync_process_env()`, and only the exec path works while the agent server is wedged. - `routers/git.py`'s set-PAT note ranked `remote_updated` above `env_updated` because "the live git process authenticates from the remote URL". Both now mean the agent works immediately, so both say so. Also simplifies a test fixture that had become a lambda calling a lambda. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…urvives Two silent failure modes found by reviewing my own diff. **The exec amplification is on the rotation path, not the boot pass.** The bound was written where it looked needed — the fleet one-shot — and the caller that actually needed it is `propagate_github_pat`, which gathers over the WHOLE FLEET with no bound of its own. This change takes each agent from one exec to three, so a 50-agent rotation is 150 execs against the fixed 6-thread `_docker_executor` the whole backend shares (`to_thread` draws from it too, #2433), each holding its thread for up to the in-container timeout. The bound moves to `scrub_git_remote_tokens` itself, so every caller — rotation, start hook, boot pass — inherits one, and a concurrent boot pass cannot out-run a rotation. **A bare `create_task` is GC-collectable mid-flight** (#1083). The boot sweep sleeps 20 s before doing any work and runs exactly once per boot, so a collected task is a remediation that silently never happened. It now holds a strong ref, the same remedy `main._first_run_seed_task` documents — and the scheduling moved into the service, because inlining it in `_schedule_staggered_services` pushed that phase past the 100-line threshold `test_1028_lifespan_phases.py` guards. Splitting is what that guard asks for. Four tests, including that the start hook did NOT inherit #2069's `auto_sync_enabled` gate — on a fleet of read-only template agents, that gate would skip most of them. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found reviewing the diff. A `skill_sources` row may hold userinfo of its own — `_adopt_legacy_clone` writes rows with no validation, and `reject_embedded_credentials` only guards NEW writes — and moving the platform PAT from the URL into `http.extraHeader` left BOTH in play for such a row: libcurl's basic auth from the URL and our header. That is exactly the double-credential shape ent#347 documented as rejected by GitHub, spelled differently, so those rows would have kept failing after a change that reads as though it fixed them. `_clone_target` composes the two halves and carries the rule neither can: **when we are going to send a credential, the URL must not carry one** — and when we are NOT, the stored userinfo is left exactly as it is, because for a row whose own token is its only credential, stripping it is this issue's own cardinal sin applied to the skills library. It resolves `_normalized_url` through `self`, not the class: that is the seam the ent#237/ent#332 fixtures override per instance to let a local fixture repo path through, and a `cls.`-qualified call bypasses it silently — which is how 23 tests went red on the first attempt. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… released Two findings from `/review` over the branch diff. **The guard had this issue's own failure mode, one level up.** It scanned `ast.JoinedStr` only, so the same defect written as concatenation, `%`-format or `.format()` would have shipped green — and a guard with a known blind spot is the ent#314 lesson again (a scan that walks one shape is not a scan). It now flattens five expression shapes, and the self-test is parametrized over SIX literal spellings of the defect — including the exact strings the three deleted producers used — plus five shapes it must stay quiet on, four of them the scrubbers this change deliberately keeps. Still zero allowlist entries. **The boot lease was taken and left to expire.** `acquire` never waits, so a losing worker has already returned by the time the winner finishes; holding the lease for its full 900 s TTL bought nothing and silently skipped the sweep on a deliberate restart inside the window — the one moment an operator is most likely to want it. Released in a `finally`. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o read `/autoplan` reads this ledger before planning, so both entries are written for that reader. 1. **A guard that knows only the CURRENT spelling of a defect certifies the next spelling of it.** Both halves of this issue hit it: registering the credential helper under its filename produces a helper that silently never runs — a fleet-wide fetch/push outage that every source-level assertion agrees is fine — and the AC1 producer guard walked f-strings only, so the same defect written as concatenation or `%`-format would have shipped green under a guard whose whole job was to prevent it. 2. **A comment that states the premise a change inverts is load-bearing documentation, and it goes stale silently.** Four sat downstream of #1967's "`.env` is not where git authenticates from" — including an accepted-risk block that this change is the changelog for. None is reachable by grepping the symbol that changed; they name a property of the system, so grep the SENTENCE. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hip is not a boundary The two blockers `/review` + `/cso --diff` raised against this branch, both reproduced before they were fixed. [C1] On a hardened install the sweep did nothing and reported success. `agent_full_capabilities=false` gives the container RESTRICTED_CAPABILITIES, which withholds DAC_OVERRIDE and FOWNER — so root inside it is subject to ordinary permission checks against the 0700 `developer`-owned home. Two things followed. The strip's config writes are both `|| true` while `scrubbed` was incremented unconditionally, so a write that failed still reported a removal: a false `remotes_scrubbed` is strictly worse than a refusal, because the operator acts on it and stops looking. And when the exec could not traverse the tree at all, `find` enumerated nothing, the probe failed, and the report was all zeros with exit 0 — which is exactly what a healthy, already-clean agent reports, so nothing distinguished "nothing to do" from "could not even look". The counter is now earned: the sweep re-reads each key after writing and counts only what is really gone, and whatever survived is a refusal, which already alarms. `root_readable` is the discriminator for the second, and an unreadable pass files its own operator-queue family — separate from the refusal, because the two need different operator action and one daily-stable id would let whichever fired first suppress the other all day. Nothing is destroyed in either case; the token stays where it already was. What changes is that the platform stops certifying a remediation that did not happen. [C2] This branch documented a security property the platform does not have. "The agent cannot rewrite what platform git executes" was written into `architecture/security.md`, `requirements/security.md` and three code comments — while the base image's own `usermod -aG sudo developer` grants `NOPASSWD:ALL`, so an agent that wants to rewrite the helper or `/etc/gitconfig` can sudo and do it. The claim was already false on dev; this branch is what promoted it from an unstated assumption into a documented invariant in the file the next reviewer reads first. All five sites now say what root ownership actually buys — that nothing rewrites those paths BY ACCIDENT — and a source guard keeps the falsified sentence out, paired with a test that fails if the sudo grant is ever removed, which would make the corrected wording the stale one. Each fix is mutation-checked: reverting the earned counter, the readability probe, the alarm branch or the corrected wording each turns a named test red. The permission-bit tests skip under uid 0, where the bits mean nothing, and both arms carry a positive control. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AndriiPasternak31
force-pushed
the
AndriiPasternak31/ent615
branch
from
September 14, 2026 23:31
7c262fb to
a3e0a51
Compare
Contributor
Author
|
@vybe — rebased onto Two decisions are yours and still unanswered from the claim comment:
Left as a draft: Not self-merging (SOC 2 separation of duties) — needs someone else's review. |
AndriiPasternak31
marked this pull request as ready for review
September 14, 2026 23:41
Contributor
|
merge-train: this PR is on today's train. Body patched — No code was pushed to your branch. Validation found no critical: the helper speaks the credential protocol over stdin, the only surviving URL builder is Two items for your call, neither blocking the merge:
Also noted and accepted: the sweep spawns at |
3 tasks
dolho
added a commit
that referenced
this pull request
Sep 15, 2026
… 903 dev lines into the split packages The modify/delete conflicts on `routers/settings.py` and `services/git_service.py` are resolved by DELETING dev's monolith copies and re-porting every hunk dev added to them since the fork into the file that now owns it, symbol by symbol, with each function's body checked equal to dev's modulo package qualification: git_service (one dev commit, ent#615 / #2757 — the fleet-PAT fix): - `_AUTH_PATTERNS` marker -> conflicts.py - `_git_remote_url` removed, `_remote_seturl_subcommand` docstring, `_credentialless_remote_url`, `rebind_origin_and_push` (root push + credential in the exec env), `update_remote_pat` (env write, not URL) -> remotes.py - the credential-helper install + embedded-token sweep block (`write_container_github_pat`, both alarms, `scrub_git_remote_tokens`, the fleet sweep, `spawn_git_remote_token_scrub`, all `_SCRUB_*`) -> NEW token_scrub.py (remotes.py would otherwise sit at 821 lines, over the threshold the split exists for) - `_agent_can_push`, `_agent_has_write_credentials` docstring, `sync_to_github`, `reset_to_main_preserve_state` -> sync.py - `initialize_git_in_container` (seeds before writing a remote) -> provisioning.py Package `__init__` re-exports every new name; the duplicate `REBIND_PUSH_TIMEOUT_S` the hunk would have introduced is dropped. settings (five dev commits — #2715, #2619, #2707, #2741, #2739): - 11 changed routes replaced in place across flags/credentials/ integrations/generic - 11 new symbols placed beside their dev-order predecessors; the #2715 Resend/Gemini routes + their two helpers go to NEW provider_keys.py (credentials.py would otherwise reach 1,045 lines), included on the package router right after `credentials` and before `generic` - `_ANTHROPIC_KEY_ALIASES` / `_adopt_after_instance_key_removed` reached from generic.py through the sibling module object, per the package rule ops: `_format_model_name`'s #2739 `claude-fable-5-1` entry lands in `ops_costs_service.py`, where the split moved the function; the #2726 test imports from there. Dev's tests that patch monolith attributes are re-pointed the way the split re-pointed every earlier one: the ent#615 exec recorder is installed on each execing sibling and `_detect_git_dir` on `gitignore`; #2572's `db` fake on `credentials` and `generic`; ent#553's source read on `flags`; #1677's emitter allowlist and the ent#615 source reads on `token_scrub`. `_PRE_SPLIT_ROUTES`' post-split allowlist records the six #2715 routes; the git_service import-surface pin drops `_git_remote_url` (gone by design) for its ent#615 replacements. Content conflicts: `backend.md` (dev's facts under the package names), `test_ent123_tokenless_clone.py` (dev's helper patch, on `gs.sync`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
This was referenced Sep 21, 2026
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
2 participants
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.
Fixes abilityai/trinity-enterprise#615
Journey Impact: extends: J09
Trinity persisted every agent's git remote as
<scheme>://oauth2:<PAT>@<host>/<org>/<repo>.git. That put the fleet-wide GitHubtoken in two places:
.git/configon the workspace volume — at rest, readable by the agent's ownBashtool for the life of the container. The larger surface, and the one anargv-only fix leaves untouched.
git-remote-https's argv onevery fetch and push, including the 60 s sync-health poll, so it was in the
process table ~2×/min even for an agent that ran no git itself.
The argv half is the one with a platform-side sink:
ps→ the agent server'sreaped-cmdline logging → Vector → the host log files → the logs API → another
agent's LLM context.
ent#292closed the last hop of that chain and was ratedP0. This closes the cause.
👉 Review this first: the harvest
This is the highest-risk third of the diff. It writes a credential into every
container it touches, and it runs fleet-wide at boot.
The rule it implements is: no path may strip a credential it has not already
replaced. One class of agent has its only credential inside its own origin URL —
POST /{agent}/git/initializewrites anagent_git_configrow and pushes with theresolved (often global) platform PAT, but bakes no git env, persists no per-agent
row and writes no
.env. For that agent, a strip-first sweep is not a scrub; it isdestruction of its last credential.
Three regression tests are the ones to read:
test_ent615_token_free_remotes.py::TestTheSweep::test_it_REFUSES_rather_than_stripping_what_it_cannot_replacestartup.sh's per-restart rewrite preserves an orphan's only credential::TestStartupShRewriteIsConditional::test_an_orphans_only_credential_is_PRESERVEDGITHUB_PAT::TestConfigurePushRemoteInputIsUnchanged::test_a_tokenless_agent_stays_blackholed_after_a_sweepThe harvest lands in
/home/developer/.trinity/git-credential(0600, ignoredcontents-only by
.trinity/*per #2070) — the helper's last rung, so it onlyever serves an agent with nothing else. Deliberately not
.envasGITHUB_PAT:startup.shexports that name asGH_TOKEN/GITHUB_TOKEN(authenticating the whole
ghCLI and REST API) andconfigure_push_remotegatesthe ent#123 push blackhole on it, so writing it would be the ent#162 class applied
fleet-wide. And not the per-agent DB row either —
_apply_git_env_from_dbwouldbake that into
GITHUB_PATon the next recreate, arriving at the same grant onestep later.
The mechanism
A git credential helper: git speaks the credential protocol to it over
stdin/stdout, which never reaches argv and is never persisted. Zero changes at the
~42 raw
subprocess.run(["git", …])sites inagent_server/routers/git.py, and itcovers platform-, agent- and
docker exec-initiated git identically.Three properties are load-bearing, and each is proven by execution (git 2.50.1)
rather than asserted:
trinity, not as the filename. Git prependsgit-credential-to any helper value that is not an absolute path, so thefilename resolves to
git-credential-git-credential-trinity— a command thatdoes not exist. The helper silently never runs, and with credential-less URLs
that is a fleet-wide fetch/push outage no source-only CI can see.
TRINITY_GIT_BASE_URLis a runtime value, so acredential.<base>.helperbaked at image-build time could only ever name github.com and a self-hosted
install would get no helper at all. A bare-hostname compare would silently
refuse the gitea harness (git feeds
host=trinity-gitea-dev:3000); a suffixcompare would be an exfiltration primitive.
.env→ baked env → harvest file — the inverse ofstartup.sh's ladder,deliberately.
startup.shruns at boot, where bakedConfig.Envis thefreshly-recreated truth; the helper runs in steady state, where
Config.Envis immutable without a recreate and
.envcarries the token a no-restart changejust injected. Baked-env-first would make a global rotation (bug: global GitHub PAT rotation never reaches agents — propagation skips agents without .env and never re-templates the git remote #1967) a silent
no-op until the old token is revoked.
Four producers, not two
git_service._git_remote_url_credentialless_remote_urlstartup.shCLONE_URLtemplate_service.clone_github_reposubprocess.runon the backend hostskill_service._authenticated_urlorigin, leaving the platform PAT at rest in/data/skills-library/*/.git/config— on the~/trinity-datahost bind mount, and so in every backup and snapshot of itGuarded over both trees (
src/backend+docker/base-image, the ent#314two-tree lesson) with zero allowlist entries, matching the producing shape —
a secret interpolated into URL userinfo — across six spellings (f-string,
concatenation,
%,.format()), not the literaloauth2:, which would flag thescrubbers this change deliberately keeps (every pre-existing volume still holds
a token).
Also in this PR
_agent_has_write_credentials's stated premise — "the global tier isdeliberately excluded; a global PAT never reaches a tokenless container's
remote" — is what this change falsifies.
_agent_can_pushconsults itfirst and the helper's ladder second (by exit code, only for agents the cheap
tiers call tokenless), so the platform stops telling an agent that can push that
it has no write credentials.
workspace volume — but runs as root with the user's PAT in the exec
environment, so
/proc/<pid>/environis unreadable by thedeveloper-uid agent.PROTECTED_KEYSgains theGIT_CONFIG_KEY_*/VALUE_*slots (the count wasalready guarded; the payload was not) plus
GIT_CONFIG_NOSYSTEM, which wouldotherwise disable the
/etc/gitconfigthe helper is registered in.start_agent_internalhook(without bug: the fleet-wide .gitignore merge never runs at agent creation — in-container auto-sync commits .trinity/ runtime state before any Push can migrate #2069's
auto_sync_enabledgate) +startup.sh's conditionalrewrite. No new recurring service — nothing produces a token URL any more.
What
/review+/cso --diffcaught, and what changed because of itBoth blockers were reproduced before they were fixed, and each fix is
mutation-checked — reverting it turns a named test red.
agent_full_capabilities=falsethe container getsRESTRICTED_CAPABILITIES,which withholds
DAC_OVERRIDE, so root inside it is subject to ordinarypermission checks against the 0700
developer-owned home. The strip's twoconfig writes are
|| truewhile the counter incremented unconditionally — soa failed write still reported a removal, and a false
remotes_scrubbedisstrictly worse than a refusal, because the operator acts on it and stops
looking. And when the exec could not traverse the tree at all, the report was
all zeros with exit 0 — exactly what a healthy already-clean agent reports.
The counter is now earned (re-read after write; whatever survived is a
refusal, which already alarms), and
root_readableis the discriminator forthe second, with its own operator-alert family — sharing the refusal's
daily-stable id would let whichever fired first suppress the other all day.
Nothing was ever destroyed by this; the token stayed where it already was. What
changes is that the platform stops certifying a remediation that did not happen.
See the last bullet below. The sentence is corrected in all five places it
reached, and a source guard keeps it out, paired with a test that fails if the
sudo grant is ever removed — which would make the corrected wording the stale
one.
Three findings were rated informational and are not fixed here (unquoted
git_dirreaching a root exec, root-side writes following symlinks, execbounding) — an adversarial pass refuted the first two as privilege escalations,
since
developeralready holds passwordless sudo, and the third is bounded bythe shared semaphore this branch adds.
Not closed — stated rather than discovered
GITHUB_PATstays in thecontainer env and
.env, and invoking the helper returns it. Root ownership ofthe helper is neither confidentiality nor a boundary:
developerholdsNOPASSWD:ALL, so an agent that wants to rewrite the helper or/etc/gitconfigcan
sudo. What it buys is that nothing rewrites them by accident. Statedthis way deliberately — an earlier draft of this branch wrote the stronger claim
into the security area file, which is where the next reviewer would have built
on it. Structural fix:
trinity-enterprise#558(AAuth).lever already exists and is unused: per-agent, repo-scoped PATs via
agent_git_config.github_pat_encrypted(bug: per-agent GitHub PAT (#347) never propagates to an existing container — git remote keeps an empty password, all pushes fail #1264's machinery).forward only; pre-existing backups and log archives still hold what the URLs
carried. Runbook:
docs/migrations/GIT_REMOTE_TOKEN_SCRUB_2026-09.md. If thesweep reports a
gitmodules_hits, rotation becomes mandatory — a token in atracked
.gitmodulesis already committed and pushed.Two decisions still open — @vybe
Neither blocks review; both are yours, and both are unanswered from the claim
comment.
ent#292— the last hop of this same chain, the logline — was rated P0. This is the cause of that chain, and it is live on
every install. It changes urgency and release sequencing, not the fix.
only: every pre-existing workspace volume, backup and log archive still
holds what the old URLs carried. My recommendation is unchanged — rotate
after this lands, so the rotation does not have to race the remediation —
except that a token found in a tracked
.gitmodulesmakes rotationmandatory rather than advisable, because it is already committed and pushed.
The sweep reports exactly that as
gitmodules_hitsand logs an error namingthe agent; the runbook says so.
Testing
tests/unit/test_ent615_token_free_remotes.py+test_ent615_credential_helper_parity.py— 84 tests. Most of the first executes shell: the helper and the sweep are shell scripts, and a Python assertion
over their source text passes against a script that cannot run — which is
exactly the CRIT-1 failure above.
startup.sh's functions are extracted and runtoo, because
/verify-local's agent stage bootslocal:test-echoand neverreaches them.
.env-first ladder, theexact-host check, the harvest-before-strip order, the root exec or the
credential-less re-point each turns specific tests red.
/verify-local(no--skip-agent): PASS on all 7 stages — unit, backendimage build + import smoke, agent base-image build, boot + health, a real
agent container exercised, integration (70 passed, 13 skipped). Run on the
pre-rebase tree; the rebase onto
devand the review fixes after it arecovered by the unit comparison below, not by a second full verify-local.
local:test-echoand never reaches the git paths, so this was run separately):the helper is installed root:root 0755 and registered in
/etc/gitconfigastrinity; the agent uid cannot write either;git credential fillresolvesfrom
.env, prefers it over a stale bakedGITHUB_PAT(CRIT-3), serves aself-hosted
host:port, refuses a foreign host, and emitsTRINITY_GIT_NO_CREDENTIAL+ "could not read Username … terminal promptsdisabled" when nothing resolves (AC4). AC2 with a positive control: a
deliberately token-bearing remote puts its token on git-child argv 3 times
(the detector fires); the credential-less remote shows 0 argv hits and 6
helper invocations — the credential flowed over stdin. Deliberately not
measured by sampling
ps, where a green result is equally consistent with thedetector never firing.
devworktree of the same tip.This branch: 16013 passed, 8 failed.
dev: 15910 passed, 8 failed. Thetwo failure sets are identical (
diffof the sortedFAILEDlines isempty) — all 8 are IPv6-mapped-address and
sys.modules-isolation tests infiles this branch does not touch. Zero regressions, +103 net new passing
tests. A baseline worktree, not a stash, because a stash leaves committed
changes in the "baseline".
Still to verify on a live stack — stated, not hidden
Three acceptance items need a real fleet PAT and a sibling stack, which the
in-image proof above could not stand in for. They are the ones a reviewer with
an instance should exercise, and I have not:
including a rotation without a recreate. The unit tests pin the plumbing
and the ladder; only a real remote proves the credential is accepted.
and the negative: make the harvest write fail and confirm the URL is left
intact with an operator-queue entry.
surfaced through
GET /api/agents/{n}/git/statusand the logs API, carriesno userinfo. This is the actual win of the issue, and it is the one thing
neither the unit suite nor the in-image proof observes.
The test-catalog entry is
trinity-dev#26, separate on purpose — a.claudegitlink bump is invisible under
diff.ignoresubmodules alland ejects a PR fromthe merge train.
🤖 Generated with Claude Code