diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 05a7ed1d1e..196de6f879 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -333dadfd8bdab3102fd23df6c4b2fb31675c69cd +422a7406f76e57bdb3e89a08971242e4a8a465bb diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 86fbed4c04..3e8f52cbdf 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4775,9 +4775,8 @@ async fn maintain_git_credentials_inner(db: &Pool) -> error::Result<() // Least-recently-checked first, so a pass that runs out of budget resumes // where it stopped instead of re-checking the same head of the list forever. // A repository with no recorded credential sorts last: it has nothing to - // rotate, yet a remote whose URL carries a token on a host that is not - // GitLab still costs a probe every pass and never records a check, so put - // first it would hold the head of the list ahead of the tokens that expire. + // rotate, and it never records a check, so put first it would hold the head + // of the list ahead of the tokens that expire. let rows = sqlx::query!( r#"SELECT ws.workspace_id, ws.git_sync FROM workspace_settings ws diff --git a/backend/tests/fixtures/git_sync_fork_credential.sql b/backend/tests/fixtures/git_sync_fork_credential.sql index 5f860129c0..1c1d8bd1e1 100644 --- a/backend/tests/fixtures/git_sync_fork_credential.sql +++ b/backend/tests/fixtures/git_sync_fork_credential.sql @@ -46,8 +46,7 @@ INSERT INTO workspace_settings (workspace_id, git_sync) VALUES -- The resource each repository entry names, all pointing at the same repository. -- The errored fork carries its token in the URL, the way a repository configured --- by hand does: nothing stores a credential for it, so only its recorded check --- knows which host it talks to. +-- by hand does: a plain remote, whatever the chain above it holds. INSERT INTO resource (workspace_id, path, value, resource_type) VALUES ('parent-ws', 'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'), ('fork-ws', 'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'), diff --git a/backend/tests/git_sync_fork_credential.rs b/backend/tests/git_sync_fork_credential.rs index 8bde262c99..c87de259f5 100644 --- a/backend/tests/git_sync_fork_credential.rs +++ b/backend/tests/git_sync_fork_credential.rs @@ -39,28 +39,19 @@ async fn credential_status_is_a_workspaces_own(db: Pool) -> anyhow::Re } /// The host a repository talks to is declared when its credential is stored, and -/// travels with the credential down the fork chain. A repository whose token -/// rides in its URL has no stored credential, so its host is known only from the -/// check that introspected the token. +/// travels with the credential down the fork chain. /// -/// Read from the recorded status alone, a fork answered with the default +/// Read from the recorded status instead, a fork answered with the default /// provider until its own check ran, which is long enough to register a webhook -/// against the wrong receiver. Read from the credential alone, a URL-token -/// repository answered with the default forever. +/// against the wrong receiver. #[sqlx::test(fixtures("git_sync_fork_credential"))] async fn the_provider_comes_from_the_credential_and_reaches_forks( db: Pool, ) -> anyhow::Result<()> { assert_eq!( repo_provider(&db, "parent-ws", REPO).await, - GitProvider::GitLab, - "with nothing stored, the host the check recorded is the answer" - ); - assert_eq!( - repo_provider(&db, "fork-ws", REPO).await, GitProvider::GitHub, - "a fork with neither a credential to resolve nor a check of its own \ - answers the default" + "with nothing stored there is no declaration to read, so the default stands" ); set_git_credential( @@ -94,9 +85,9 @@ async fn the_provider_comes_from_the_credential_and_reaches_forks( ); assert_eq!( repo_provider(&db, "errored-fork-ws", REPO).await, - GitProvider::GitLab, - "a token carried in the URL is held by nobody, so the parent's credential \ - is not consulted and the recorded check alone names the host" + GitProvider::GitHub, + "a token written into the URL makes the repository a plain remote: the \ + parent's credential is not consulted and no host is declared" ); Ok(()) } diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 592dbd8cf8..7396375998 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -34389,9 +34389,6 @@ components: type: array items: type: string - token_fingerprint: - type: string - description: one-way digest identifying which credential this status describes rotatable: type: boolean description: whether this workspace renews the credential itself diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index d3d34d9fc4..341aa5df67 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -436,11 +436,6 @@ pub struct GitCredentialStatus { pub expires_at: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub scopes: Vec, - /// Which credential this status describes, as a one-way digest. Lets a later - /// check tell "the token I know about stopped working" from "someone put a - /// different credential here", which look identical at the API otherwise. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub token_fingerprint: Option, /// Whether *this workspace* renews the credential. That needs a scope which /// permits it (`api` or `self_rotate`) and a credential this workspace holds: /// a token carried in the repository URL is the operator's to manage, and one diff --git a/docs/git-sync-gitlab-setup.md b/docs/git-sync-gitlab-setup.md index d9a714da11..6e736dee37 100644 --- a/docs/git-sync-gitlab-setup.md +++ b/docs/git-sync-gitlab-setup.md @@ -81,21 +81,18 @@ App installation token. Storing it this way keeps it out of the variables API an out of every fork's own storage; it is not a boundary against the admins of those workspaces. -A URL with the token written into it keeps working, whether it sits in the -resource or in a secret variable the resource points at (`"url": "$var:..."`). -Windmill reports its expiry on the repository but does not renew it: that token -is yours to manage, as it is for a GitHub repository authenticated the same way. -Forking copies such a URL with its value, so two workspaces would hold the same -token each believing it issued it, and presenting an already-rotated one to -GitLab costs the whole token family. Use the **GitLab** button to hand the token -to Windmill if you want it renewed. +A URL with the token written into it, in the resource or in a secret variable +the resource points at (`"url": "$var:..."`), is a plain git remote: it syncs on +deploy and by polling, and nothing else. Windmill does not know the token is +there, so it registers no webhook, opens no merge request, and neither reports +nor renews its expiry. Use the **GitLab** button to hand the token to Windmill +if you want any of that. ## Expiry and renewal -Windmill reads `expires_at` from the token itself and shows it on the repository -in the workspace's git sync settings, for every repository including the ones -whose token sits in the URL. A token Windmill holds it also renews: within three -weeks of expiry it rotates it through GitLab's own +Windmill reads `expires_at` from the token it holds and shows it on the +repository in the workspace's git sync settings. Within three weeks of expiry it +rotates the token through GitLab's own `POST /personal_access_tokens/self/rotate`, stores the replacement, and verifies it. Only the token can rotate itself, so one without `api` (or `self_rotate`) is a permanent warning rather than something Windmill can fix.