fix(secret-rotator): keep the previous gitea token through the ESO propagation window
ci/woodpecker/push/build-ci-images Pipeline was successful
ci/woodpecker/push/build-ci-images Pipeline was successful
Second review pass (no blocking issues). The important one: the reap deleted EVERY other prefix token right after store, including the previous run's token — which is still the live credential in the k8s Secret until ESO syncs the new value and source-controller re-reads it. Deleting it immediately opens an availability window (ESO refreshInterval + reconcile lag) where Flux's stored token has just been revoked. Since this mode exists specifically to avoid stranding Flux, that window shouldn't exist. Now the reap keeps the TWO newest tokens (the one just stored + the previous) and deletes only older ones. tokens_with_prefix returns (id, name) so the caller can order by recency; the previous token survives until the next quarterly run, long after ESO has synced. Also: - doc-note that PUSH is the load-bearing gate check: for a public verify_repo, git-upload-pack returns 200 for any valid token, so pull barely gates — don't set GITEA_REQUIRE_PUSH=false against a public repo. - fetch tokens with limit=50 (single effective page) and note the count stays tiny given keep-two reaping, so pagination can't silently miss old tokens. - tidy the cleanup error message connector. clippy -D warnings clean; 14 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -96,16 +96,21 @@ pub fn token_allows(
|
||||
}
|
||||
}
|
||||
|
||||
/// Names of the user's tokens that start with `prefix`. Used to reap old
|
||||
/// rotated tokens after a new one is stored.
|
||||
pub fn token_names_with_prefix(
|
||||
/// (id, name) of the user's tokens whose name starts with `prefix`, used to
|
||||
/// reap old rotated tokens. The id is returned so the caller can order by
|
||||
/// recency (gitea ids are monotonic) and keep the newest ones.
|
||||
///
|
||||
/// The result is fetched with a large per_page so it is effectively a single
|
||||
/// page. The prefixed-token count stays tiny in practice — the caller keeps
|
||||
/// only the two newest, so accumulation never approaches a page boundary.
|
||||
pub fn tokens_with_prefix(
|
||||
agent: &ureq::Agent,
|
||||
base_url: &str,
|
||||
user: &str,
|
||||
password: &str,
|
||||
prefix: &str,
|
||||
) -> Result<Vec<String>, Box<dyn Error>> {
|
||||
let url = format!("{}/api/v1/users/{user}/tokens", base_url.trim_end_matches('/'));
|
||||
) -> Result<Vec<(i64, String)>, Box<dyn Error>> {
|
||||
let url = format!("{}/api/v1/users/{user}/tokens?page=1&limit=50", base_url.trim_end_matches('/'));
|
||||
let mut resp = agent
|
||||
.get(&url)
|
||||
.header("Authorization", &basic(user, password))
|
||||
@@ -114,9 +119,14 @@ pub fn token_names_with_prefix(
|
||||
let list = v.as_array().ok_or_else(|| GiteaError("tokens list not an array".into()))?;
|
||||
Ok(list
|
||||
.iter()
|
||||
.filter_map(|t| t["name"].as_str())
|
||||
.filter(|n| n.starts_with(prefix))
|
||||
.map(str::to_string)
|
||||
.filter_map(|t| {
|
||||
let name = t["name"].as_str()?;
|
||||
if name.starts_with(prefix) {
|
||||
Some((t["id"].as_i64().unwrap_or(0), name.to_string()))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
|
||||
@@ -280,8 +280,11 @@ fn rotate_gitea_token() -> Result<String, Box<dyn Error>> {
|
||||
// covers all of them.
|
||||
let outcome = (|| -> Result<(), Box<dyn Error>> {
|
||||
// Safety gate: the new token must actually work before it replaces the
|
||||
// old one. Pull is mandatory; push is required when the consumer writes
|
||||
// back (Flux image-automation).
|
||||
// old one. NOTE: for a PUBLIC verify_repo, git-upload-pack returns 200
|
||||
// for any valid token regardless of scope, so the pull check is weak —
|
||||
// PUSH (git-receive-pack, which requires write) is the load-bearing
|
||||
// check. Do NOT set GITEA_REQUIRE_PUSH=false against a public repo, or
|
||||
// the gate degrades to "is this any valid token".
|
||||
let can_pull = gitea::token_allows(&agent, &gitea_url, &user, &new, &verify_repo, "git-upload-pack")?;
|
||||
let can_push = if require_push {
|
||||
gitea::token_allows(&agent, &gitea_url, &user, &new, &verify_repo, "git-receive-pack")?
|
||||
@@ -303,7 +306,7 @@ fn rotate_gitea_token() -> Result<String, Box<dyn Error>> {
|
||||
match gitea::delete_token(&agent, &gitea_url, &user, &password, &name) {
|
||||
Ok(()) => {
|
||||
return Err(format!(
|
||||
"new token '{name}' {e}; deleted it, existing token left in place"
|
||||
"new token '{name}': {e}; deleted it, existing token left in place"
|
||||
)
|
||||
.into())
|
||||
}
|
||||
@@ -317,13 +320,20 @@ fn rotate_gitea_token() -> Result<String, Box<dyn Error>> {
|
||||
}
|
||||
}
|
||||
|
||||
// Reap old rotated tokens (same prefix), keeping the one just stored. Non-
|
||||
// fatal: the rotation already succeeded; leftover old tokens are a tidiness
|
||||
// issue, not a correctness one.
|
||||
// Reap OLD rotated tokens — but keep the two newest: the one just stored
|
||||
// AND the immediately-previous one. The previous token is still the live
|
||||
// credential in the k8s Secret until ESO syncs the new value and
|
||||
// source-controller re-reads it; deleting it here would strand Flux for
|
||||
// that propagation window. Keeping it until the next rotation (a quarter
|
||||
// later, long after ESO has synced) closes that window. Non-fatal: the
|
||||
// rotation already succeeded; leftover tokens are tidiness, not correctness.
|
||||
let mut reaped = 0usize;
|
||||
match gitea::token_names_with_prefix(&agent, &gitea_url, &user, &password, &name_prefix) {
|
||||
Ok(names) => {
|
||||
for n in names.iter().filter(|n| *n != &name) {
|
||||
match gitea::tokens_with_prefix(&agent, &gitea_url, &user, &password, &name_prefix) {
|
||||
Ok(mut toks) => {
|
||||
// newest first by id (gitea ids are monotonic); keep [0]=new and
|
||||
// [1]=previous, delete the rest.
|
||||
toks.sort_by_key(|(id, _)| std::cmp::Reverse(*id));
|
||||
for (_, n) in toks.iter().skip(2) {
|
||||
if gitea::delete_token(&agent, &gitea_url, &user, &password, n).is_ok() {
|
||||
reaped += 1;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user