feat: project source-control bindings for member intent starts#333
feat: project source-control bindings for member intent starts#333JWThewes wants to merge 7 commits into
Conversation
Members could not start intents without a personal GitHub/GitLab connection: the orchestrator resolved the starting user's token and init-ws failed asynchronously (checkout_failed / intent_branch_push_failed). Replace per-user runtime auth with project-level source-control bindings: - Owners/admins bind each repository once, choosing GitHub App (per-repo installation discovery) or explicitly confirmed github-oauth / gitlab-oauth delegation at step 1 of the create-space flow; the App path needs no personal connection at all (repo discovery runs on App-JWT-minted metadata-read installation tokens via /github/app/*). - Bindings hold opaque credential refs in a new DynamoDB table, never tokens, and auto-invalidate when the delegator disconnects, loses scopes, or leaves the project. - Intent starts validate bindings up front (409 SOURCE_CONTROL_NOT_READY with per-repo reasons) instead of failing mid-run. - AgentCore runtime is token-free: git operations obtain short-lived credentials from a credential-broker lambda (IAM-scoped to the runtime role, validates execution liveness) via a throwaway GIT_ASKPASS helper; tokens no longer enter invocation payloads or durable history. - Provider API operations (branches, PRs, issues) route through a source-control service lambda; per-provider API proxy routes and the platform-wide GitHub auth mode are removed. Also bumps js-yaml 4.2.0 -> 4.3.0 in the lockfile (GHSA-52cp-r559-cp3m, pre-existing advisory that now trips the pre-commit audit). Closes #332
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ng (#332) - Clear-text logging (js/clear-text-logging, alerts 17-20): error objects in the credential-resolution path can carry provider-derived text. Logging sites in credential-broker and source-control now emit only allowlisted error-code constants via loggableErrorCode() and no longer log error.message. - Polynomial regex (js/polynomial-redos): canonicalRepo trimmed leading and trailing slashes with /^\/+|\/+$/g over caller-supplied repo refs; replaced with a linear-time scan.
An installation lacking Workflows: Read & write previously failed binding verification outright. There are valid reasons to withhold that permission, so treat it as recommended instead of required: the binding verifies, the reduced capability (workflows: none) is recorded on the binding, token mints skip the ungranted permission (GitHub rejects mints requesting more than the installation grants), and the project settings page shows a warning that the agent cannot touch .github/workflows/.
jeromevdl
left a comment
jeromevdl
left a comment
There was a problem hiding this comment.
Gitlab token refresh
When a GitLab access token is near expiry, two git operations can try to refresh it at the same time.
lambda/shared/git-token.js:125 reads the stored token and refreshes it, but does not prevent another request from doing the same. GitLab replaces the refresh token on a successful refresh. That means the first request succeeds and saves the new token pair, while the second request uses the now-invalid old refresh token and gets invalid_grant.
This PR can trigger that situation because it asks the credential broker for credentials separately for each git operation (lambda/agentcore/git-auth.js:42). Parallel construction lanes can therefore refresh the same GitLab connection simultaneously. The failed request is treated as a broken credential and invalidates the project’s source-control binding (lambda/shared/source-control-bindings.js:281), blocking repository work until an owner rebinds it.
Please make refresh single-flight per GitLab connection, or re-read the stored token when a refresh fails before marking the binding invalid. Add a test that issues two credential-broker requests concurrently for an expired GitLab token.
Project-bound GitLab API operations lost the existing 401 refresh-and-retry behavior.
The GitLab provider retries only when ctx.onRefresh is provided (lambda/shared/git-providers/gitlab.js:54). The new source-control path invokes providers with only { token } (lambda/source-control/index.js:365), so an early/clock-skew/revoked access-token 401 is not refreshed. Instead it invalidates the binding as provider_unauthorized (lambda/shared/source-control-bindings.js:277). The old personal GitLab handler still has onRefresh; the PR’s project-bound path does not.
Validation
✅ Tested with gitlab end-to-end, clone => branch => PR all worked, didn't face any of the edge case above
Review feedback on #333 (two issues, one root cause: a transient credential failure permanently invalidated the project binding). 1. GitLab refresh race: refresh tokens are one-time-use, and parallel construction lanes each request credentials separately, so two concurrent refreshes of the same connection burned each other — the loser got invalid_grant, which the broker escalated to binding invalidation. - ensureFreshGitToken now single-flights refreshes per connection within a container (shared in-flight promise). - On invalid_grant, refreshGitlabToken re-reads the stored pair before failing: if it rotated, another request (possibly another container) won the race — return its token instead of throwing. Only a genuinely revoked (un-rotated) pair still fails. 2. Project-bound GitLab operations lost the personal handler's 401 refresh-and-retry: providers were invoked with a bare {token} ctx, so an early/clock-skew 401 invalidated the binding as provider_unauthorized. resolveBindingCredential now returns a refresh callback for gitlab-oauth bindings (force-refresh past the rejected token, race-safe via staleToken) and the source-control lambda wires it into ctx.onRefresh for both operations and live validation. The broker response shape is unchanged. Tests: concurrent-refresh single-flight, cross-container race recovery, genuine revocation still failing, staleToken force/rotation semantics, and the requested end-to-end test firing two concurrent broker requests for an expired GitLab token (one refresh, both succeed, no binding invalidation).
…po permissions Bind-and-verify failed with INSUFFICIENT_REPOSITORY_ACCESS for a GitHub App installed on all repositories with Contents: Read & write. GET /repos returns user-authority-shaped `permissions` (push/admin) that are absent or all-false for an installation token, so access.canWrite is not a reliable signal on the App path. Write authority is already proven before the probe runs: validateGitHubAppInstallation requires contents:write on the installation, and the repo-scoped contents:write mint succeeds only when GitHub grants it. Keep the GET /repos probe (it still fails the binding when the installation cannot see the repository — 404/403) but stop requiring canWrite from it on github-app bindings, in both verifyGitHubAppBinding and the live validate-project check.
|
@jeromevdl I addressed your comments and also fixed the GH App issue. after pulling this, the previously-failed binding should verify on the next "Bind and verify" click there's no config change needed on the GitHub App side |
3 participants
Closes #332
Problem
Members could not start intents without a personal GitHub/GitLab connection. The intent created and started fine (201/202), but the run failed asynchronously: the orchestrator resolved the starting user's personal token (
no_connection), and init-ws hard-failed withcheckout_failed/intent_branch_push_failed. Raw git tokens were also snapshotted into AgentCore payloads and durable execution history (no delete API, 90-day retention).Approach
Per-user runtime auth is replaced with project-level source-control bindings:
/github/app/status|reposroutes), or explicitly confirmed GitHub OAuth / GitLab OAuth delegation of the binder's connection. Bindings live in a new DynamoDB table holding only opaque credential references, never tokens, and auto-invalidate when the delegator disconnects, loses scopes, or leaves the project.409 SOURCE_CONTROL_NOT_READYwith per-repo reasons instead of failing mid-run.GIT_ASKPASShelper. Tokens no longer enter invocation payloads or durable history.Testing
tsc -bclean.validate+fmt -check -recursiveclean (backend-config init errors pre-exist).Notes for reviewers