fix(claw): a borrowed watch credential carries the account's login

`resolve_watch_token` handed the adapter a bare string, so the client
could only ever try it as a token — and a `secret_kind` is a label the
human typed at `identity_save`, not a fact: a login password saved as a
token resolved as one, 401'd on every poll, and the watch stayed dead
with nothing said.

`resolve_watch_credential` returns the value *and* the login of the
account it came from (`WatchCredential`), the client cache keys on both
(one value sent two ways is two credentials), and the gitea client is
built with `with_login`, so the adapter's 401 retry can reach the
instance's own Basic-auth form.

A row token or the forge's environment variable names no login, so those
tiers resolve exactly as before: `WatchCredential::bare`, `login: None`.
`resolve_watch_token` stays as the token half, for callers with no use
for the login.
This commit is contained in:
kosmos authored and Valentin Haudiquet committed 2026-11-08 17:54:15 +01:00
1 parent d31dfeae27
commit 6468ae9638
2 files changed
+163 -39

No files matched your search

+152 -32
View File
@@ -12,9 +12,10 @@
//! (spec §1). Reading the tables every pass is what makes a watch added //! (spec §1). Reading the tables every pass is what makes a watch added
//! mid-flight (chat tool, HTTP route) take effect without a restart. //! mid-flight (chat tool, HTTP route) take effect without a restart.
//! Repo-watch clients are constructed on first use and cached per //! Repo-watch clients are constructed on first use and cached per
//! `(forge, host, resolved token)` — resolved through //! `(forge, host, resolved credential)` — resolved through
//! [`resolve_watch_token`], so a blank row borrows the matching //! [`resolve_watch_credential`], so a blank row borrows the matching
//! account identity; account clients are built fresh every pass, //! account identity; account clients are built fresh every pass,
//! so a rotated identity secret is live on the next tick. Cadence //! so a rotated identity secret is live on the next tick. Cadence
//! belongs to the daemon's loop; the source is one pass, nothing more. //! belongs to the daemon's loop; the source is one pass, nothing more.
@@ -33,23 +34,66 @@ use rhema_rsd::{Clock, Dispatcher, Event, KeyFn, SystemClock};
use crate::watch_store::{AccountWatchRecord, ClawStore, WatchRecord}; use crate::watch_store::{AccountWatchRecord, ClawStore, WatchRecord};
use rhema_forge_integration::{ForgeKind, resolve_token}; use rhema_forge_integration::{ForgeKind, resolve_token};
/// The credential a watch polls with: the value the forge takes as a
/// bearer-style token, and the login that owns it when an account
/// identity supplied the value.
///
/// The login is why this is a struct and not a string: an instance that
/// refuses the value in the token header can be retried as Basic auth,
/// which needs `user:password` — see `GiteaForge::with_login`. A row's
/// stored token and the forge's environment variable name no login, so
/// they carry `None`.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct WatchCredential {
/// The secret itself.
pub token: String,
/// The login that owns `token`, when an account identity supplied it.
pub login: Option<String>,
}
impl WatchCredential {
/// A credential from a bare value: a row token or an environment
/// variable, neither of which names a login.
pub fn bare(token: impl Into<String>) -> Self {
Self {
token: token.into(),
login: None,
}
}
/// Whether nothing resolved at all: the client then polls anonymous,
/// and the forge — not the daemon — is what says no.
pub fn is_empty(&self) -> bool {
self.token.is_empty()
}
}
/// The cache key of a forge client: everything that changes the client /// The cache key of a forge client: everything that changes the client
/// is the forge kind, its host, and the token it authenticates with. /// is the forge kind, its host, and the credential it authenticates
/// with — the login included, because one value sent two ways is two
/// credentials.
#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ClientKey { struct ClientKey {
forge: String, forge: String,
host: Option<String>, host: Option<String>,
token: String, token: String,
login: Option<String>,
} }
/// The forge client one watch RECORD polls through: the kind's /// The forge client one watch RECORD polls through: the kind's
/// production base unless the record names a `host`. Callers pass the /// production base unless the record names a `host`. Callers pass the
/// credential already resolved through [`resolve_watch_token`] (the /// credential already resolved through [`resolve_watch_credential`] (the
/// account watches pass the identity store's secret directly), so a /// account watches pass the identity store's secret directly), so an
/// blank token here means nothing resolved anywhere: the client polls /// empty token here means nothing resolved anywhere: the client polls
/// anonymous, and the poll fails loudly at the forge, not silently. /// anonymous, and the poll fails loudly at the forge, not silently.
pub fn record_client(kind: ForgeKind, host: Option<&str>, token: &str) -> Arc<dyn Forge> { pub fn record_client(
let resolved = resolve_token(|name| std::env::var(name), kind, Some(token)).unwrap_or_default(); kind: ForgeKind,
host: Option<&str>,
credential: &WatchCredential,
) -> Arc<dyn Forge> {
let resolved = resolve_token(|name| std::env::var(name), kind, Some(&credential.token))
.unwrap_or_default();
match kind { match kind {
ForgeKind::GitHub => { ForgeKind::GitHub => {
let mut forge = GitHubForge::new(resolved); let mut forge = GitHubForge::new(resolved);
@@ -58,10 +102,16 @@ pub fn record_client(kind: ForgeKind, host: Option<&str>, token: &str) -> Arc<dy
} }
Arc::new(forge) Arc::new(forge)
} }
ForgeKind::Gitea => Arc::new(GiteaForge::new( ForgeKind::Gitea => {
host.unwrap_or_default().to_owned(), let mut forge = GiteaForge::new(host.unwrap_or_default().to_owned(), resolved);
resolved, // The login rides the client: a credential the instance
)), // refuses in the token header is retried as Basic auth (see
// `GiteaForge::with_login`).
if let Some(login) = &credential.login {
forge = forge.with_login(login.clone());
}
Arc::new(forge)
}
ForgeKind::GitLab => { ForgeKind::GitLab => {
let mut forge = GitLabForge::new(resolved); let mut forge = GitLabForge::new(resolved);
if let Some(host) = host { if let Some(host) = host {
@@ -86,24 +136,49 @@ pub fn record_client(kind: ForgeKind, host: Option<&str>, token: &str) -> Arc<dy
/// account) is visible. A matching account that holds a password is /// account) is visible. A matching account that holds a password is
/// skipped without a word: a password is not a poll credential, and /// skipped without a word: a password is not a poll credential, and
/// the per-pass alternative is a log line per watch per tick. /// the per-pass alternative is a log line per watch per tick.
///
/// A derived credential carries the account's login too. The store's
/// `secret_kind` is a label its human typed at `identity_save`, not a
/// fact about the value, so a login password saved as a token resolves
/// here — and the login is what lets the forge client retry it as Basic
/// auth (`GiteaForge::with_login`) instead of 401-ing every tick. A row
/// token or an environment variable names no login and carries `None`.
pub fn resolve_watch_credential(
accounts: &dyn IdentityStore,
kind: ForgeKind,
host: Option<&str>,
stored: &str,
) -> WatchCredential {
if let Some(token) = resolve_token(|name| std::env::var(name), kind, Some(stored)) {
return WatchCredential::bare(token);
}
accounts
.list()
.ok()
.unwrap_or_default()
.into_iter()
.filter(|account| account.service.eq_ignore_ascii_case(kind.name()))
.filter(|account| host_matches(host, account.url.as_deref()))
.find_map(|account| {
let secret = accounts.secret(&account.id).ok()??;
(secret.kind == SecretKind::Token).then(|| WatchCredential {
token: secret.value,
login: account.username,
})
})
.unwrap_or_default()
}
/// The token half of [`resolve_watch_credential`]: what a caller with no
/// use for the login — a cache key's text, a test — needs.
pub fn resolve_watch_token( pub fn resolve_watch_token(
accounts: &dyn IdentityStore, accounts: &dyn IdentityStore,
kind: ForgeKind, kind: ForgeKind,
host: Option<&str>, host: Option<&str>,
stored: &str, stored: &str,
) -> Option<String> { ) -> Option<String> {
resolve_token(|name| std::env::var(name), kind, Some(stored)).or_else(|| { let credential = resolve_watch_credential(accounts, kind, host, stored);
accounts (!credential.is_empty()).then_some(credential.token)
.list()
.ok()?
.into_iter()
.filter(|account| account.service.eq_ignore_ascii_case(kind.name()))
.filter(|account| host_matches(host, account.url.as_deref()))
.find_map(|account| {
let secret = accounts.secret(&account.id).ok()??;
(secret.kind == SecretKind::Token).then_some(secret.value)
})
})
} }
/// Whether `account_url` names the same instance a watch with `host` /// Whether `account_url` names the same instance a watch with `host`
@@ -182,24 +257,25 @@ impl ForgeSource {
// table would be skipped in `detect` before reaching here. // table would be skipped in `detect` before reaching here.
unreachable!("detect filters unknown forge kinds before client lookup") unreachable!("detect filters unknown forge kinds before client lookup")
}; };
let resolved = resolve_watch_token( let credential = resolve_watch_credential(
self.accounts.as_ref(), self.accounts.as_ref(),
kind, kind,
record.host.as_deref(), record.host.as_deref(),
&record.token, &record.token,
) );
.unwrap_or_default();
// The RESOLVED credential, not the row's: two blank-token // The RESOLVED credential, not the row's: two blank-token
// watches whose identities differ must not share a client. // watches whose identities differ must not share a client, and
// neither must one value answerable as a token and as a password.
let key = ClientKey { let key = ClientKey {
forge: record.forge.clone(), forge: record.forge.clone(),
host: record.host.clone(), host: record.host.clone(),
token: resolved.clone(), token: credential.token.clone(),
login: credential.login.clone(),
}; };
let mut clients = self.clients.lock().unwrap_or_else(PoisonError::into_inner); let mut clients = self.clients.lock().unwrap_or_else(PoisonError::into_inner);
clients clients
.entry(key) .entry(key)
.or_insert_with(|| record_client(kind, record.host.as_deref(), &resolved)) .or_insert_with(|| record_client(kind, record.host.as_deref(), &credential))
.clone() .clone()
} }
} }
@@ -309,8 +385,19 @@ impl ForgeSource {
} }
// Fresh client every pass: a rotated secret is live on the next // Fresh client every pass: a rotated secret is live on the next
// tick, and one client build per account per pass is nothing // tick, and one client build per account per pass is nothing
// next to the poll itself. // next to the poll itself. The login rides it: a password saved
let client = record_client(kind, watch.host.as_deref(), &secret.value); // under the token kind still polls, as Basic auth.
let login = self
.accounts
.get(&watch.account_id)
.map_err(|err| SourceError::Backend(format!("account store: {err}")))?
.and_then(|account| account.username);
let credential = WatchCredential {
token: secret.value,
login,
};
let client = record_client(kind, watch.host.as_deref(), &credential);
let since = self let since = self
.account_cursors .account_cursors
.lock() .lock()
@@ -440,6 +527,39 @@ mod tests {
assert_eq!(resolved.as_deref(), Some("acct-tok")); assert_eq!(resolved.as_deref(), Some("acct-tok"));
} }
/// A derived credential carries the account's login: it is what lets
/// the forge client retry a secret the instance refuses as a token
/// (a login password saved under the token kind) as Basic auth.
#[test]
fn a_borrowed_credential_carries_the_accounts_login() {
let accounts = rhema_identity::VolatileIdentityStore::new();
accounts
.put(
rhema_identity::AccountDraft::new(
"kosmos",
"gitea",
rhema_identity::Secret::new(rhema_identity::SecretKind::Token, "hunter2"),
)
.with_url("https://git.example")
.with_username("kosmos"),
)
.expect("account lands");
let derived =
resolve_watch_credential(&accounts, ForgeKind::Gitea, Some("https://git.example"), "");
assert_eq!(derived.token, "hunter2");
assert_eq!(derived.login.as_deref(), Some("kosmos"));
// A row token names no login: there is nothing to retry with.
let row = resolve_watch_credential(
&accounts,
ForgeKind::Gitea,
Some("https://git.example"),
"row-tok",
);
assert_eq!(row, WatchCredential::bare("row-tok"));
}
/// The derived identity is instance-scoped: a self-hosted watch /// The derived identity is instance-scoped: a self-hosted watch
/// never borrows an account saved against another instance, and a /// never borrows an account saved against another instance, and a
/// production-base watch accepts only a bare or public-host URL. /// production-base watch accepts only a bare or public-host URL.
+11 -7
View File
@@ -23,7 +23,7 @@ use std::collections::HashMap;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex, PoisonError}; use std::sync::{Arc, Mutex, PoisonError};
use crate::forge::{record_client, resolve_watch_token}; use crate::forge::{record_client, resolve_watch_credential};
use crate::watch_store::ClawStore; use crate::watch_store::ClawStore;
use rhema_agent_tools::AllowAll; use rhema_agent_tools::AllowAll;
use rhema_autonomous_agent::{Handler, SessionContext, async_trait}; use rhema_autonomous_agent::{Handler, SessionContext, async_trait};
@@ -119,19 +119,23 @@ impl ClawHandler {
// The same chain the poll resolved: row token, env var, then // The same chain the poll resolved: row token, env var, then
// the matching account identity — keyed on the resolved value, // the matching account identity — keyed on the resolved value,
// so a credential rotation (or a saved account) is a new client // so a credential rotation (or a saved account) is a new client
// on the next dispatch. // on the next dispatch. The login rides along: an instance that
let resolved = resolve_watch_token( // refuses the value in the token header takes it as Basic auth
// (`GiteaForge::with_login`).
let credential = resolve_watch_credential(
self.accounts.as_ref(), self.accounts.as_ref(),
kind, kind,
record.host.as_deref(), record.host.as_deref(),
&record.token, &record.token,
) );
.unwrap_or_default(); let cache_key = format!(
let cache_key = format!("{}|{:?}|{}", record.forge, record.host, resolved); "{}|{:?}|{}|{:?}",
record.forge, record.host, credential.token, credential.login
);
let mut clients = self.clients.lock().unwrap_or_else(PoisonError::into_inner); let mut clients = self.clients.lock().unwrap_or_else(PoisonError::into_inner);
Ok(clients Ok(clients
.entry(cache_key) .entry(cache_key)
.or_insert_with(|| record_client(kind, record.host.as_deref(), &resolved)) .or_insert_with(|| record_client(kind, record.host.as_deref(), &credential))
.clone()) .clone())
} }