Repository navigation
fix(git-sync): fork-to-own and repo binding use the saved personal GitHub token (#3164) - #3235
Conversation
…tHub token (#3164) Fork-to-own and the post-creation repo binding required a GitHub token in the form even when the user had a personal token saved in Settings; the create path already resolved it (resolve_github_pat, tier per_user) but only used it to read the template. - ForkToOwnRequest.github_pat / BindAgentRepoRequest.github_pat are optional; a supplied token still wins and is still charset-validated. - Omitted: fork-to-own uses the resolver's per_user token; binding reads the caller's own saved token by user id (never the agent's current per-agent token). Persisted as the agent's per-agent PAT, tier `fork`, as before. - The platform token is never a fork or bind identity: with no personal token the request is a named 400 FORK_PAT_REQUIRED pointing at Settings (ent#162 Decision 2 — the global PAT must not land in a per-agent row). - A token refusal raised while using the saved token says so (token_source "saved", message naming Settings). - UI: SavedGithubTokenField shows "Using your saved GitHub token" with an explicit override, presence read from GET /api/users/me/github-pat; the field is required only when nothing is saved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train (2026-10-05, evening run): not on this train. It rides the next one once fixed. CI is green at Blocking
Needs a decision before it lands
Smaller
What held up: precedence (form, then saved, never platform), the field names on both sides, the raw-colour baseline (both files shrank), and the frontend never receives the token value. |
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Independent /review + /validate-pr pass at 512381f. Same head as vybe's merge-train comment, so his four items are still open. I confirmed each one and have two additions.
Confirmed, with evidence
- git.py:1101 crash (vybe #1). Reproduced with your own
bindfixture:bind.saved = SAVED, then_bindraisesRuntimeError("header value b'Bearer ghp_saved_personal' is illegal"). The result isAttributeError: 'NoneType' object has no attribute 'get_secret_value'at git.py:1101. The form-token control passes.
Addition: the audit row and the idempotency release are both skipped, and there's a second problem. main.py has no catch-all exception handler, so the AttributeError's chained traceback ("During handling of the above exception…") logs the original exception text unscrubbed. That's theBearer <PAT>echo the comment above line 1101 says this branch exists to keep out of the Vector log.
Fix:scrub_secret_and_urls(str(e), user_pat), plus a saved-token test for that branch. The assert I used:
assert e.value.status_code == 500 and SAVED not in str(e.value.detail) - Override + empty field sends the saved token (vybe #2). BindRepoPanel.vue:311 and CreateAgentModal.vue:549 can't see
overriding(SavedGithubTokenField.vue:63). Emitting the override state and guarding on!token && (!hasSaved || overriding)closes it. One mount spec per form for override+empty, please. - Admin bind (D1) and agent-key fork (D2). Both reachable as described: OwnedAgentByName admits admin (dependencies.py:1755), and POST /api/agents is require_role("creator") with no agent_name check on fork_to_own. For D1, note that resolve_github_pat's own docstring says resolution "keys on ownership only, never on a calling/sharing user" (settings_service.py:1019). Keying the bind fallback on owner == caller would match that.
New
- docs/memory/architecture/agent-lifecycle.md:191 still says the user PAT is header-validated at the model for both request types. The saved token is a plain str from db.get_user_github_pat and never passes
_validate_pat_secret. It's validated at save time instead (strip + GitHub round-trip, users.py:225), and the doc should say that. The ent#109 bind section there also doesn't mention the fallback yet. SAVED_TOKEN_TIERSincludesper_agent, but both create-path resolver calls pass onlyowner_id(crud.py:706,724), so that tier can't occur there. Either drop it or add a comment saying why it's safe if a future caller passesagent_name.
What holds up: reverting the crud and router hunks turns 7 of the 14 new tests red. Adding "global" to SAVED_TOKEN_TIERS turns the global-tier test red. 380 related fork/bind/PAT unit tests pass locally.
The two CANCELLED checks are benign. The first frontend-e2e run was superseded under cancel-in-progress when the ui label and the PR open fired two seconds apart. The superseding run on the same SHA is green, and report only runs on schedule.
…ork, no empty override (#3164 review) - Bind's unexpected-error handler scrubs with the token actually used; `body.github_pat` is None on the saved path and raised before the scrub, skipping the audit row and the idempotency release. - Bind falls back to the saved token only when the caller owns the agent; an admin binding someone else's agent must type a token. - Fork-to-own refuses the saved token for an agent key (an agent key resolves to its owner, so it could create a repo in the owner's account). - SAVED_TOKEN_TIERS is per_user only; per_agent cannot occur on the create path and would hand an agent's identity to a user-owned repo. - SavedGithubTokenField emits `update:overriding`; BindRepoPanel and CreateAgentModal refuse an override submitted empty instead of sending the saved token. The input is BaseInput. - /me/github-pat adds `usable` (present AND decryptable); the forms read it. - Docs: agent-lifecycle, agent-repo-binding and template-processing describe the optional token and where the saved one is validated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed vybe's 10-05 train notes and @AndriiPasternak31's review in Blocking
Decisions (I took the narrower option in both cases — say if you want the other one)
Smaller
The touched test files were run locally (19 backend + 37 frontend). The full suite runs in CI. 🤖 Generated with Claude Code |
|
merge-train (2026-10-07): validated at
|
|
merge-train (2026-10-07): pushed |
Summary
Fork-to-own and the post-creation repo binding always asked for a GitHub token in the form, even when the user had a personal token saved in Settings. The create path already resolved that token (
resolve_github_pat, tierper_user), but it only used it to read the template.ForkToOwnRequest.github_patandBindAgentRepoRequest.github_patare now optional. A supplied token still wins and is still charset-validated; a blank token is still rejected.per_usertoken (crud._apply_fork_to_own).db.get_user_github_pat). It never uses the agent's current per-agent token, which is the agent's identity, not the binding person's.fork, exactly as before.FORK_PAT_REQUIRED, which points at Settings. Otherwise the destination would belong to the platform account, and the per-agent row would hold the global PAT (ent#162 Decision 2).token_source: "saved"and the message says to update it in Settings.SavedGithubTokenField.vueis used in the create form and the bind panel. It reads presence only from the existingGET /api/users/me/github-pat(no new endpoint, never the value). With a saved token it shows "Using your saved GitHub token" and an explicit "Use a different token" override, and the request carries no token. Without one, the field is required as before.Changes
models.py,services/agent_service/{crud,fork_to_own}.py,routers/git.py: the optional token, the fallback,FORK_PAT_REQUIREDand the saved-token wordingstores/auth.js(fetchGithubPatStatus),SavedGithubTokenField.vue(new),CreateAgentModal.vue,BindRepoPanel.vueraw-color-baseline.json: two entries lowered by hand (BindRepoPanel 46 → 41, CreateAgentModal 135 → 126) with a_3164_note. Not regenerated, because that would drop therefrozenblock.feature-flows/github-import-intents.md,requirements/github.mdTest Plan
tests/unit/test_3164_fork_uses_saved_pat.py: 14 pass. It covers the AC cases (a) saved token + no form token forks with the saved one, (b) global-only gives 400FORK_PAT_REQUIREDand nothing reaches GitHub, (c) a form token beats a saved one, and (d) the same three for binding. It also covers the saved-token error wording and the models.-p randomly --randomly-seed=12345.savedGithubToken.mount.spec.js: 7 mounted tests. The 5 payload tests fail without the wiring.npm run test:unit: all passed, including the ratchets.verify-localstack, fake tokens seeded directly):FORK_PAT_INVALID,token_source: saved, saved-token wording, and the token is not in the response;FORK_PAT_REQUIRED;configuredonly;Fixes #3164
🤖 Generated with Claude Code