Approvals through an agent token say so (closes #229) 05f1a85a2842
An unsealed approval or rejection made with an agent's token was recorded as the person alone. It now records the token's name beside them (flow_jobs.approved_via, migration 0078 for SQLite and Postgres): "approved by carol via claude-code" in the job's log and the run page, "via claude-code" in the audit log. approved_by stays the person whose account decided, as the keyless certificate's claim and others-only checks use it. Sealed approvals already refuse delegated keys (#235). Whether agent tokens may decide unsealed gates at all stays Andy's call. Test: a rejection through the API with an agent token. Closes #229
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
agent:claude@andymccall <[email protected]> · keyless, under your grant for changes,refs,seal · key 63ea6bd8594d
- When
- · 2026-10-18 01:00 UTC
- Signature
- ◈ sealed by agent:claude@andymccall
- Hash
05f1a85a284253adc4e796dba60b0a0556198b5e2be230687802a9a5ee3af821- Depends on
- 0b20f1ec4007 3b1bc27fe805 417184c73f4e 42fa95540468 47e374020b59 5d90e0e045e2 60f11b71a740 965b950f9b96 b644e1757711 b64e2537d702 d23464c0ac60 dc59d1b56c3d
- Depended on by
- b1860d6090ec 4a68f1064b08 e97500b70f9a
- In bookmarks
- api-unknown-fields cancel-landed-runs diffusion-pin-bookmark draft-ready main oh-comment-body-file proposal-list-fast
- In tags
- v0.6.0 v0.6.1 v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 40 atoms, 2 tree operations, 7 files
Browse the tree at this change
added crates/hub/migrations-pg/0078_flow_approved_via.sql +3 −0
| 1 | + | -- Approvals through an agent token say so (oxygenhub#229): the token's |
|
| 2 | + | -- name, beside approved_by (the person, whose account decided). |
|
| 3 | + | ALTER TABLE flow_jobs ADD COLUMN approved_via TEXT NOT NULL DEFAULT ''; |
added crates/hub/migrations/0078_flow_approved_via.sql +3 −0
| 1 | + | -- Approvals through an agent token say so (oxygenhub#229): the token's |
|
| 2 | + | -- name, beside approved_by (the person, whose account decided). |
|
| 3 | + | ALTER TABLE flow_jobs ADD COLUMN approved_via TEXT NOT NULL DEFAULT ''; |
modified crates/hub/src/api/mod.rs +7 −2
| … | 324 unchanged lines |
||
| 325 | 325 | pub token: String, |
|
| 326 | 326 | /// The personal access token's id, when one is used. |
|
| 327 | 327 | pub token_id: Option<i64>, |
|
| 328 | + | /// The token is an agent's (oxygenhub#183): what it does says so. |
|
| 329 | + | pub agent: bool, |
|
| 328 | 330 | } |
|
| 329 | 331 | ||
| 330 | 332 | impl Caller { |
|
| … | 16 unchanged lines |
||
| 347 | 349 | ||
| 348 | 350 | pub async fn caller(app: &Shared, headers: &HeaderMap) -> Result<Caller, ApiError> { |
|
| 349 | 351 | let Some(t) = oxy_server::bearer_token(headers) else { |
|
| 350 | - | return Ok(Caller { account: None, scopes: vec![], site: false, token: String::new(), token_id: None }); |
|
| 352 | + | return Ok(Caller { account: None, scopes: vec![], site: false, token: String::new(), token_id: None, agent: false }); |
|
| 351 | 353 | }; |
|
| 352 | 354 | if app.config.auth.push_tokens.iter().any(|x| auth::mac_equal(x, t)) { |
|
| 353 | 355 | let admin = db::accounts::accounts(&app.db).await?.into_iter().find(|a| a.is_admin); |
|
| … | 3 unchanged lines |
||
| 357 | 359 | site: true, |
|
| 358 | 360 | token: "site token".into(), |
|
| 359 | 361 | token_id: None, |
|
| 362 | + | agent: false, |
|
| 360 | 363 | }); |
|
| 361 | 364 | } |
|
| 362 | 365 | let token = db::accounts::token_by_hash(&app.db, &auth::token_hash(t)).await?; |
|
| … | 7 unchanged lines |
||
| 370 | 373 | site: false, |
|
| 371 | 374 | token: tok.name.clone(), |
|
| 372 | 375 | token_id: Some(tok.id), |
|
| 376 | + | agent: tok.agent, |
|
| 373 | 377 | }) |
|
| 374 | 378 | } |
|
| 375 | 379 | _ => Err(ApiError(StatusCode::UNAUTHORIZED, "unknown or expired token".into())), |
|
| … | 1918 unchanged lines |
||
| 2294 | 2298 | let me = c.require("repo:write")?.clone(); |
|
| 2295 | 2299 | let (_, row, _) = reach(app, &c, owner, name, Access::Read).await?; |
|
| 2296 | 2300 | let run = db::flow::run(&app.db, row.id, number).await?.ok_or_else(|| ApiError(StatusCode::NOT_FOUND, "no such run".into()))?; |
|
| 2297 | - | let said = crate::flow::decide(app, &row, &run, job, &me, approve).await.map_err(|m| ApiError(StatusCode::CONFLICT, m))?; |
|
| 2301 | + | let via = c.agent.then_some(c.token.as_str()); |
|
| 2302 | + | let said = crate::flow::decide(app, &row, &run, job, &me, approve, via).await.map_err(|m| ApiError(StatusCode::CONFLICT, m))?; |
|
| 2298 | 2303 | Ok(Json(serde_json::json!({ "number": number, "job": job, "decision": if approve { "approved" } else { "rejected" }, "said": said }))) |
|
| 2299 | 2304 | } |
|
| 2300 | 2305 | ||
| … | 384 unchanged lines |
modified crates/hub/src/db/flow.rs +10 −5
| … | 154 unchanged lines |
||
| 155 | 155 | /// Who approved or rejected it, and when. |
|
| 156 | 156 | pub approved_by: String, |
|
| 157 | 157 | pub approved_ms: Option<i64>, |
|
| 158 | + | /// The agent token it was decided through, if one (oxygenhub#229). |
|
| 159 | + | #[sqlx(default)] |
|
| 160 | + | pub approved_via: String, |
|
| 158 | 161 | /// The environment it deploys to, or empty (oxygenflow#59). |
|
| 159 | 162 | pub environment: String, |
|
| 160 | 163 | /// The approval attestation that released it, when sealed (#235). |
|
| … | 884 unchanged lines |
||
| 1045 | 1048 | exit_code = NULL, error = '', receipt = '', receipt_hash = '', log_count = 0, also_read = '', receipt_attest = '', |
|
| 1046 | 1049 | receipt_signer = '', receipt_note = '', runner_id = NULL, waiting = '', placed = '', peak_disk_mib = NULL, |
|
| 1047 | 1050 | peak_memory_mib = NULL, approval = CASE WHEN approval = '' THEN '' ELSE 'waiting' END, approved_by = '', |
|
| 1048 | - | approved_ms = NULL, approval_seal = '' WHERE id = $1", |
|
| 1051 | + | approved_ms = NULL, approval_seal = '', approved_via = '' WHERE id = $1", |
|
| 1049 | 1052 | ) |
|
| 1050 | 1053 | .bind(id) |
|
| 1051 | 1054 | .execute(&mut *tx) |
|
| … | 118 unchanged lines |
||
| 1170 | 1173 | ||
| 1171 | 1174 | /// Approve job `id` if it is still waiting for a person (oxygenflow#57); |
|
| 1172 | 1175 | /// whether it was. |
|
| 1173 | - | pub async fn approve_job(pool: &Pool, id: i64, who: &str, seal: &str) -> sqlx::Result<bool> { |
|
| 1176 | + | pub async fn approve_job(pool: &Pool, id: i64, who: &str, seal: &str, via: &str) -> sqlx::Result<bool> { |
|
| 1174 | 1177 | let r = sqlx::query( |
|
| 1175 | - | "UPDATE flow_jobs SET approval = 'approved', approved_by = $1, approved_ms = $2, approval_seal = $3 \ |
|
| 1178 | + | "UPDATE flow_jobs SET approval = 'approved', approved_by = $1, approved_ms = $2, approval_seal = $3, approved_via = $5 \ |
|
| 1176 | 1179 | WHERE id = $4 AND status = 'queued' AND approval = 'waiting'", |
|
| 1177 | 1180 | ) |
|
| 1178 | 1181 | .bind(who) |
|
| 1179 | 1182 | .bind(now_ms()) |
|
| 1180 | 1183 | .bind(seal) |
|
| 1181 | 1184 | .bind(id) |
|
| 1185 | + | .bind(via) |
|
| 1182 | 1186 | .execute(pool) |
|
| 1183 | 1187 | .await?; |
|
| 1184 | 1188 | Ok(r.rows_affected() > 0) |
|
| 1185 | 1189 | } |
|
| 1186 | 1190 | ||
| 1187 | 1191 | /// Note who rejected job `id` (it is then finished as cancelled). |
|
| 1188 | - | pub async fn note_rejection(pool: &Pool, id: i64, who: &str) -> sqlx::Result<()> { |
|
| 1189 | - | sqlx::query("UPDATE flow_jobs SET approved_by = $1, approved_ms = $2 WHERE id = $3") |
|
| 1192 | + | pub async fn note_rejection(pool: &Pool, id: i64, who: &str, via: &str) -> sqlx::Result<()> { |
|
| 1193 | + | sqlx::query("UPDATE flow_jobs SET approved_by = $1, approved_ms = $2, approved_via = $4 WHERE id = $3") |
|
| 1190 | 1194 | .bind(who) |
|
| 1191 | 1195 | .bind(now_ms()) |
|
| 1192 | 1196 | .bind(id) |
|
| 1197 | + | .bind(via) |
|
| 1193 | 1198 | .execute(pool) |
|
| 1194 | 1199 | .await?; |
|
| 1195 | 1200 | Ok(()) |
|
| … | 37 unchanged lines |
modified crates/hub/src/flow.rs +30 −9
| … | 198 unchanged lines |
||
| 199 | 199 | name: &str, |
|
| 200 | 200 | who: &db::accounts::Account, |
|
| 201 | 201 | approve: bool, |
|
| 202 | + | via: Option<&str>, |
|
| 202 | 203 | ) -> Result<String, String> { |
|
| 203 | - | decide_with(app, row, run, name, who, approve, None).await |
|
| 204 | + | decide_with(app, row, run, name, who, approve, Through { seal: None, via }).await |
|
| 204 | 205 | } |
|
| 205 | 206 | ||
| 207 | + | /// How a decision arrived: with a sealed approval's ref, or through an |
|
| 208 | + | /// agent token (by its name). |
|
| 209 | + | #[derive(Clone, Copy, Default)] |
|
| 210 | + | struct Through<'a> { |
|
| 211 | + | seal: Option<&'a str>, |
|
| 212 | + | via: Option<&'a str>, |
|
| 213 | + | } |
|
| 214 | + | ||
| 206 | 215 | /// [`decide`], from a verified seal (`Some(approval ref)`) or without one; |
|
| 207 | 216 | /// a gate that takes a sealed approval is not approved without. |
|
| 208 | 217 | async fn decide_with( |
|
| … | 3 unchanged lines |
||
| 212 | 221 | name: &str, |
|
| 213 | 222 | who: &db::accounts::Account, |
|
| 214 | 223 | approve: bool, |
|
| 215 | - | seal: Option<&str>, |
|
| 224 | + | through: Through<'_>, |
|
| 216 | 225 | ) -> Result<String, String> { |
|
| 226 | + | let Through { seal, via } = through; |
|
| 217 | 227 | let jobs = fdb::jobs(&app.db, run.id).await.map_err(|e| e.to_string())?; |
|
| 218 | 228 | let Some(job) = jobs.into_iter().find(|j| j.name == name) else { return Err(format!("run #{} has no job {name}", run.number)) }; |
|
| 219 | 229 | let approval = may_decide(app, row, run, &job, who).await?; |
|
| … | 10 unchanged lines |
||
| 230 | 240 | } |
|
| 231 | 241 | let target = format!("{}/{}#{} {name}", row.owner, row.name, run.number); |
|
| 232 | 242 | let sealed = seal.map(|r| format!(", sealed (approval {})", &r[..12.min(r.len())])).unwrap_or_default(); |
|
| 243 | + | // Through an agent's token, it says so (oxygenhub#229). |
|
| 244 | + | let by = match via { |
|
| 245 | + | Some(v) => format!("{} via {v}", who.username), |
|
| 246 | + | None => who.username.clone(), |
|
| 247 | + | }; |
|
| 248 | + | let detail = [seal.unwrap_or(""), &via.map(|v| format!("via {v}")).unwrap_or_default()] |
|
| 249 | + | .iter() |
|
| 250 | + | .filter(|s| !s.is_empty()) |
|
| 251 | + | .cloned() |
|
| 252 | + | .collect::<Vec<_>>() |
|
| 253 | + | .join(", "); |
|
| 233 | 254 | let said = if approve { |
|
| 234 | - | if !fdb::approve_job(&app.db, job.id, &who.username, seal.unwrap_or("")).await.map_err(|e| e.to_string())? { |
|
| 255 | + | if !fdb::approve_job(&app.db, job.id, &who.username, seal.unwrap_or(""), via.unwrap_or("")).await.map_err(|e| e.to_string())? { |
|
| 235 | 256 | return Err(format!("{name} was decided on meanwhile")); |
|
| 236 | 257 | } |
|
| 237 | - | db::accounts::audit(&app.db, Some(who), "flow.approved", &target, seal.unwrap_or("")).await.ok(); |
|
| 238 | - | format!("approved by {}{sealed}", who.username) |
|
| 258 | + | db::accounts::audit(&app.db, Some(who), "flow.approved", &target, &detail).await.ok(); |
|
| 259 | + | format!("approved by {by}{sealed}") |
|
| 239 | 260 | } else { |
|
| 240 | - | fdb::note_rejection(&app.db, job.id, &who.username).await.map_err(|e| e.to_string())?; |
|
| 241 | - | let why = format!("rejected by {}{sealed}", who.username); |
|
| 261 | + | fdb::note_rejection(&app.db, job.id, &who.username, via.unwrap_or("")).await.map_err(|e| e.to_string())?; |
|
| 262 | + | let why = format!("rejected by {by}{sealed}"); |
|
| 242 | 263 | let changed = |
|
| 243 | 264 | fdb::finish(&app.db, job.id, fdb::Outcome { status: "cancelled", exit_code: None, error: &why, receipt: "", receipt_hash: "" }) |
|
| 244 | 265 | .await |
|
| 245 | 266 | .map_err(|e| e.to_string())?; |
|
| 246 | 267 | report_changed(app, run.id, changed).await.ok(); |
|
| 247 | - | db::accounts::audit(&app.db, Some(who), "flow.rejected", &target, "").await.ok(); |
|
| 268 | + | db::accounts::audit(&app.db, Some(who), "flow.rejected", &target, &detail).await.ok(); |
|
| 248 | 269 | why |
|
| 249 | 270 | }; |
|
| 250 | 271 | let record = flow_core::api::Record { t: 0, stream: "flow".into(), text: said.clone() }; |
|
| … | 92 unchanged lines |
||
| 343 | 364 | let (rid, signers) = (sa.attest.repo, sa.sigs.iter().map(|s| s.key).collect::<Vec<_>>()); |
|
| 344 | 365 | let kept = app.repos.write(&repo_name, move |r, _| Ok(r.add_approval(&sa)?)).await.map_err(|e| e.to_string())?; |
|
| 345 | 366 | crate::tlog::log(app, vec![crate::tlog::Pending { kind: "approval", repo: rid, repo_row: Some(row.id), object: kept, signers }]).await; |
|
| 346 | - | decide_with(app, row, run, name, &who, given.approve, Some(&kept.to_hex())).await |
|
| 367 | + | decide_with(app, row, run, name, &who, given.approve, Through { seal: Some(&kept.to_hex()), via: None }).await |
|
| 347 | 368 | } |
|
| 348 | 369 | ||
| 349 | 370 | /// End the approvals that waited too long (oxygenflow#57): from when the |
|
| … | 2185 unchanged lines |
modified crates/hub/src/web/flow.rs +2 −2
| … | 504 unchanged lines |
||
| 505 | 505 | by: a.by, |
|
| 506 | 506 | others_only: a.others_only, |
|
| 507 | 507 | state, |
|
| 508 | - | who: j.approved_by.clone(), |
|
| 508 | + | who: if j.approved_via.is_empty() { j.approved_by.clone() } else { format!("{} via {}", j.approved_by, j.approved_via) }, |
|
| 509 | 509 | when_ms: j.approved_ms.unwrap_or(0), |
|
| 510 | 510 | may, |
|
| 511 | 511 | }); |
|
| … | 59 unchanged lines |
||
| 571 | 571 | "reject" => false, |
|
| 572 | 572 | _ => return Err(Error::bad(shell, "approve or reject")), |
|
| 573 | 573 | }; |
|
| 574 | - | match crate::flow::decide(&app, &row, &run, &job, &me.account, approve).await { |
|
| 574 | + | match crate::flow::decide(&app, &row, &run, &job, &me.account, approve, None).await { |
|
| 575 | 575 | Ok(_) => Ok(Redirect::to(&format!("/{owner}/{name}/flow/{number}")).into_response()), |
|
| 576 | 576 | Err(why) => Err(Error::bad(shell, why)), |
|
| 577 | 577 | } |
|
| … | 364 unchanged lines |
modified crates/hub/tests/flow.rs +21 −8
| … | 4392 unchanged lines |
||
| 4393 | 4393 | }; |
|
| 4394 | 4394 | assert!(heard(oxygenhub::events::topic::repo(row.id)).contains(&"push".to_string()), "the push is on the repository's topic"); |
|
| 4395 | 4395 | sqlx::query("UPDATE flow_runs SET actor = 'andy' WHERE id = $1").bind(run.id).execute(&h.app.db).await.unwrap(); |
|
| 4396 | - | let early = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true).await.unwrap_err(); |
|
| 4396 | + | let early = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true, None).await.unwrap_err(); |
|
| 4397 | 4397 | assert!(early.contains("waits for plan first"), "{early}"); |
|
| 4398 | 4398 | let run = plan_passes(run.number).await; |
|
| 4399 | 4399 | assert!(claim(&h.app).await.is_none(), "apply waits for a person, so no runner gets it"); |
|
| … | 7 unchanged lines |
||
| 4407 | 4407 | assert_eq!((told(carol.id).await, told(andy.id).await, told(bob.id).await), (1, 0, 0)); |
|
| 4408 | 4408 | let (_, page) = call(&h.app, "GET", &format!("/andy/gated/flow/{}", run.number), None, None).await; |
|
| 4409 | 4409 | assert!(page.contains("waits for approval by someone with the <em>maintain</em> role, other than andy"), "the run page says so"); |
|
| 4410 | - | let own = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &andy, true).await.unwrap_err(); |
|
| 4410 | + | let own = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &andy, true, None).await.unwrap_err(); |
|
| 4411 | 4411 | assert!(own.contains("someone other than andy"), "{own}"); |
|
| 4412 | - | let no_role = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &bob, true).await.unwrap_err(); |
|
| 4412 | + | let no_role = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &bob, true, None).await.unwrap_err(); |
|
| 4413 | 4413 | assert!(no_role.contains("takes the maintain role"), "{no_role}"); |
|
| 4414 | - | assert_eq!(oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true).await.unwrap(), "apply: approved by carol"); |
|
| 4414 | + | assert_eq!(oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true, None).await.unwrap(), "apply: approved by carol"); |
|
| 4415 | 4415 | assert_eq!(told(carol.id).await, 1, "told once"); |
|
| 4416 | 4416 | let j = claim(&h.app).await.expect("approved, it runs"); |
|
| 4417 | 4417 | assert_eq!(j["job"]["name"], "apply"); |
|
| … | 14 unchanged lines |
||
| 4432 | 4432 | let list: serde_json::Value = serde_json::from_str(&list).unwrap(); |
|
| 4433 | 4433 | assert_eq!((list[0]["environment"].as_str(), list[0]["approved_by"].as_str()), (Some("prod"), Some("carol"))); |
|
| 4434 | 4434 | ||
| 4435 | - | // Run 2: rejected through the API's rules: the job ends cancelled, the run ends. |
|
| 4435 | + | // Run 2: rejected through the API with carol's agent token: the job |
|
| 4436 | + | // ends cancelled, the run ends, and it says the agent did it |
|
| 4437 | + | // (oxygenhub#229). |
|
| 4436 | 4438 | push(); |
|
| 4437 | 4439 | let run = plan_passes(2).await; |
|
| 4438 | - | assert_eq!(oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, false).await.unwrap(), "apply: rejected by carol"); |
|
| 4440 | + | let (secret, hash) = oxygenhub::auth::new_token(); |
|
| 4441 | + | let tok = db::accounts::create_token(&h.app.db, carol.id, "claude-code", &hash, "repo:write", "", None).await.unwrap(); |
|
| 4442 | + | db::accounts::set_token_agent(&h.app.db, carol.id, tok.id, true).await.unwrap(); |
|
| 4443 | + | let (st, body) = call(&h.app, "POST", "/api/v1/repos/andy/gated/flow/runs/2/jobs/apply/reject", Some(&secret), Some("{}".into())).await; |
|
| 4444 | + | assert_eq!(st, StatusCode::OK, "{body}"); |
|
| 4445 | + | assert!(body.contains("rejected by carol via claude-code"), "{body}"); |
|
| 4439 | 4446 | let jobs = db::flow::jobs(&h.app.db, run.id).await.unwrap(); |
|
| 4440 | 4447 | let apply = jobs.iter().find(|j| j.name == "apply").unwrap(); |
|
| 4441 | - | assert_eq!((apply.status.as_str(), apply.error.as_str()), ("cancelled", "rejected by carol")); |
|
| 4448 | + | assert_eq!((apply.status.as_str(), apply.error.as_str()), ("cancelled", "rejected by carol via claude-code")); |
|
| 4449 | + | assert_eq!((apply.approved_by.as_str(), apply.approved_via.as_str()), ("carol", "claude-code")); |
|
| 4450 | + | let audit = db::accounts::audit_rows(&h.app.db, 50).await.unwrap(); |
|
| 4451 | + | assert!( |
|
| 4452 | + | audit.iter().any(|r| r.action == "flow.rejected" && r.actor == "carol" && r.detail.contains("via claude-code")), |
|
| 4453 | + | "audited with the agent" |
|
| 4454 | + | ); |
|
| 4442 | 4455 | assert!(claim(&h.app).await.is_none()); |
|
| 4443 | 4456 | ||
| 4444 | 4457 | // Run 3: nobody decides in time. |
|
| … | 98 unchanged lines |
||
| 4543 | 4556 | let run = db::flow::run(&h.app.db, row.id, run.number).await.unwrap().unwrap(); |
|
| 4544 | 4557 | ||
| 4545 | 4558 | // An environment's gate takes a sealed approval: pressing Approve is not enough. |
|
| 4546 | - | let plain = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true).await.unwrap_err(); |
|
| 4559 | + | let plain = oxygenhub::flow::decide(&h.app, &row, &run, "apply", &carol, true, None).await.unwrap_err(); |
|
| 4547 | 4560 | assert!(plain.contains(&format!("oxy seal --flow {}/andy/infra/flow/{} --job apply", h.base, run.number)), "{plain}"); |
|
| 4548 | 4561 | // What there is to sign. |
|
| 4549 | 4562 | let (st, body) = |
|
| … | 658 unchanged lines |