Workflow job tokens never put g1t to work, and mentions send g1t back to a pull request at most 10 times a day (work 0031)
11 files+143−130/11 viewed
| 1063 | 1063 | or a comment made with it runs nothing, so a workflow cannot set itself | |
| 1064 | 1064 | off. `workflow_dispatch` and [`repository_dispatch`](#repository-dispatch) | |
| 1065 | 1065 | are the exceptions, for a workflow that means to start another. | |
| 1066 | + | - It **never puts g1t to work**. A comment it posts that mentions | |
| 1067 | + | `@g1t` starts nothing, and it cannot assign an issue or a plan to g1t, | |
| 1068 | + | queue one for it, hand it work or ask it for a review. Otherwise a | |
| 1069 | + | workflow that asks g1t to fix a failing check would run again on g1t's | |
| 1070 | + | push, and ask again, without end. A step that should put g1t to work | |
| 1071 | + | uses a token of a person's own, stored as a | |
| 1072 | + | [secret](/guides/secrets-and-variables/). | |
| 1066 | 1073 | ||
| 1067 | 1074 | `permissions:` goes at the top of the workflow, for every job, or on a job, | |
| 1068 | 1075 | which then ignores the workflow's. Once either is written, every permission |
| 414 | 414 | (`ops@g1t.sh`), in URLs, in package scopes (`@g1t/platform`) and in longer | |
| 415 | 415 | names (`@g1t-bot`) are ignored. Matching ignores case. Agents mentioning | |
| 416 | 416 | `@g1t` start nothing, so | |
| 417 | − | agents cannot set each other to work this way. | |
| 417 | + | agents cannot set each other to work this way. Neither does a comment made | |
| 418 | + | with a workflow job's own token (`G1T_TOKEN`), so a workflow cannot set | |
| 419 | + | off g1t whose push sets off the workflow again; see | |
| 420 | + | [the job's token](/guides/actions/#the-jobs-token). | |
| 418 | 421 | ||
| 419 | 422 | **Who can.** People with the Write [role](/guides/access-and-roles/) or higher on the | |
| 420 | 423 | repository, members or not. Anyone else who mentions it gets a short reply | |
| ⋯ | |||
| 428 | 431 | ||
| 429 | 432 | Each comment starts one run at most; to ask again, write a new comment. | |
| 430 | 433 | ||
| 434 | + | A mention sends g1t back to a pull request it made even after it stopped | |
| 435 | + | there, or used up the repository's revisions. At most 10 mentions in a day | |
| 436 | + | send it back to the same pull request; past that, g1t replies that it | |
| 437 | + | has reached the most it takes, and the next one works a day after the | |
| 438 | + | first of those 10. | |
| 439 | + | ||
| 431 | 440 | ## The label rule | |
| 432 | 441 | ||
| 433 | 442 | Under a project's **Settings → Agents**, someone with the Maintain role or | |
| 91 | 91 | import { BACKUP_MINUTES, backupEnv, backupPace, backupSandboxName } from "./backup"; | |
| 92 | 92 | import { type ProjectSurroundings, readableSurroundings } from "./surroundings"; | |
| 93 | 93 | import { holdCredentials, pushGrant, remotePath, revokeCredentials, runCredential } from "./credentials"; | |
| 94 | − | import { buildMentionPrompt, describeThread, handleMention, planMention } from "./mentions"; | |
| 94 | + | import { buildMentionPrompt, describeThread, handleMention, jobTokenRefusal, planMention } from "./mentions"; | |
| 95 | 95 | import { instructionsFor, repoInstructions, withBlock } from "./repo-instructions"; | |
| 96 | 96 | import { cancelTask, enqueueTask, handedOverStep, selfHostedRoute, taskEnv, taskRepo } from "./self-hosted"; | |
| 97 | 97 | import { | |
| ⋯ | |||
| 2438 | 2438 | * the actor is a member or a collaborator. | |
| 2439 | 2439 | */ | |
| 2440 | 2440 | private async refusal(actor: User, repo: RepoPath): Promise<Result<never> | null> { | |
| 2441 | + | // Agent compute is never started by a workflow job's token. | |
| 2442 | + | const byJob = jobTokenRefusal(actor); | |
| 2443 | + | if (byJob) return fail("forbidden", byJob); | |
| 2441 | 2444 | const closed = await this.closedRepo(actor, repo); | |
| 2442 | 2445 | if (closed) return closed; | |
| 2443 | 2446 | if (!(await this.workspaceAllowed(repo.namespace))) { | |
| ⋯ | |||
| 2840 | 2843 | ||
| 2841 | 2844 | async delegate(actor: User, repo: RepoPath, input: DelegateInput): Promise<Result<Delegated>> { | |
| 2842 | 2845 | // Who may put agents to work here is settled before anything is opened. | |
| 2846 | + | const byJob = jobTokenRefusal(actor); | |
| 2847 | + | if (byJob) return fail("forbidden", byJob); | |
| 2843 | 2848 | const closed = await this.closedRepo(actor, repo); | |
| 2844 | 2849 | if (closed) return closed; | |
| 2845 | 2850 | if (!actor || !(await this.repoAllows(actor, repo, "run"))) return fail("forbidden", needs("run")); | |
| 3 | 3 | ||
| 4 | 4 | import type { LifecycleJob, MentionJob, Result } from "@g1t/contracts"; | |
| 5 | 5 | ||
| 6 | − | import { type MentionPorts, buildMentionPrompt, handleMention, planMention } from "./mentions.ts"; | |
| 6 | + | import { type MentionPorts, buildMentionPrompt, handleMention, jobTokenRefusal, planMention } from "./mentions.ts"; | |
| 7 | 7 | ||
| 8 | 8 | const ana = { id: "usr_ana", username: "ana", verified: true, workspaces: [{ slug: "acme", role: "member" as const }] }; | |
| 9 | 9 | ||
| ⋯ | |||
| 162 | 162 | }; | |
| 163 | 163 | return { ports, replies, started, recorded }; | |
| 164 | 164 | } | |
| 165 | + | ||
| 166 | + | test("a workflow job's token never puts g1t to work; a person's own token can", () => { | |
| 167 | + | const job = { ...ana, token: { scopes: ["repo"], job: { run_id: "run_9", job_id: "job_1" } } }; | |
| 168 | + | assert.match(jobTokenRefusal(job) ?? "", /job's token/); | |
| 169 | + | assert.equal(jobTokenRefusal({ ...ana, token: { scopes: ["repo"] } }), null); | |
| 170 | + | assert.equal(jobTokenRefusal(ana), null); | |
| 171 | + | assert.equal(jobTokenRefusal(null), null); | |
| 172 | + | }); | |
| 11 | 11 | */ | |
| 12 | 12 | import type { Comment, LifecycleJob, MentionJob, MentionsApi, Pull, RepoPath, Result, User } from "@g1t/contracts"; | |
| 13 | 13 | ||
| 14 | + | /** | |
| 15 | + | * Why a workflow job's token (`G1T_TOKEN`) may not put g1t to work, or | |
| 16 | + | * null for anyone else. A workflow that could summon an agent on a failing | |
| 17 | + | * check would start one whose push runs the workflow again, without end. | |
| 18 | + | */ | |
| 19 | + | export function jobTokenRefusal(actor: User | null | undefined): string | null { | |
| 20 | + | return actor?.token?.job | |
| 21 | + | ? "A workflow job's token (G1T_TOKEN) cannot put g1t to work. A person, or a token of their own, can." | |
| 22 | + | : null; | |
| 23 | + | } | |
| 24 | + | ||
| 14 | 25 | /** What a mention leads to. */ | |
| 15 | 26 | export type MentionPlan = | |
| 16 | 27 | | { kind: "not_member" } |
| 1 | + | -- When a mention sent g1t back to its pull request, so that no more than | |
| 2 | + | -- a day's ceiling of them do (settings.rs MAX_MENTION_REVISIONS_PER_DAY). | |
| 3 | + | ALTER TABLE agent_mentions ADD COLUMN revised_at TEXT; | |
| 4 | + | CREATE INDEX agent_mentions_revised ON agent_mentions (pull_id, revised_at) | |
| 5 | + | WHERE revised_at IS NOT NULL; |
| 490 | 490 | let repo = check!(self.repo(&a.repo, &Some(a.actor.clone())).await?); | |
| 491 | 491 | check!(writable(&repo)); | |
| 492 | 492 | check!(allowed(Some(&a.actor), &repo, Capability::Run)); | |
| 493 | + | if let Some(refused) = mentions::refuse_job_token(&a.actor) { | |
| 494 | + | return Ok(refused); | |
| 495 | + | } | |
| 493 | 496 | self.open_issue(OpenIssueArgs { | |
| 494 | 497 | actor: a.actor, | |
| 495 | 498 | repo: a.repo, |
| 23 | 23 | use worker::wasm_bindgen::JsValue; | |
| 24 | 24 | ||
| 25 | 25 | use crate::Work; | |
| 26 | + | use crate::settings::MAX_MENTION_REVISIONS_PER_DAY; | |
| 26 | 27 | use crate::lifecycle::{POLICY_ACTOR_ID, POLICY_ACTOR_NAME, made_by_g1t}; | |
| 27 | 28 | use crate::reviews::{AGENT_ID, AGENT_NAME}; | |
| 28 | 29 | use crate::rows::{NumberRow, ValueRow}; | |
| ⋯ | |||
| 388 | 389 | Ok(Some(label.to_lowercase())) | |
| 389 | 390 | } | |
| 390 | 391 | ||
| 392 | + | /// Whether `actor` mentioning `@g1t` may set it to work. g1t and other | |
| 393 | + | /// agents may not, so no agent can set another to work; neither may a | |
| 394 | + | /// workflow job's token (`G1T_TOKEN`), or a workflow that comments on a | |
| 395 | + | /// failing check would start an agent whose push runs it again, and so on | |
| 396 | + | /// without end. | |
| 397 | + | pub(crate) fn may_summon(actor: &User) -> bool { | |
| 398 | + | actor.kind != PrincipalKind::Agent | |
| 399 | + | && !actor.is_system() | |
| 400 | + | && actor.id != AGENT_ID | |
| 401 | + | && g1t_contracts::events::job_run_of(actor).is_none() | |
| 402 | + | } | |
| 403 | + | ||
| 404 | + | /// Why a workflow job's token may not put g1t to work (`may_summon`). | |
| 405 | + | pub(crate) const JOB_TOKEN_REFUSED: &str = | |
| 406 | + | "A workflow job's token (G1T_TOKEN) cannot put g1t to work. A person, or a token of their own, can."; | |
| 407 | + | ||
| 408 | + | /// A refusal for an actor acting with a workflow job's token, which may | |
| 409 | + | /// not start agents by any path: queueing an issue, assigning a plan's or | |
| 410 | + | /// handing one over. | |
| 411 | + | pub(crate) fn refuse_job_token<T>(actor: &User) -> Option<Outcome<T>> { | |
| 412 | + | g1t_contracts::events::job_run_of(actor).map(|_| Outcome::fail(FailureCode::Forbidden, JOB_TOKEN_REFUSED)) | |
| 413 | + | } | |
| 414 | + | ||
| 415 | + | /// The start of the day `mention_revision` counts back over, from `now`. | |
| 416 | + | fn day_before(now: u64) -> String { | |
| 417 | + | rfc3339(now.saturating_sub(24 * 60 * 60 * 1000)) | |
| 418 | + | } | |
| 419 | + | ||
| 391 | 420 | impl Work { | |
| 392 | 421 | /// Records a comment's mention of `@g1t`, if it makes one, for | |
| 393 | − | /// the runner to take when it hears of the comment. g1t mentioning | |
| 394 | − | /// itself is not recorded, so no agent can set another to work. | |
| 422 | + | /// the runner to take when it hears of the comment. What agents and | |
| 423 | + | /// workflow jobs say is not recorded (`may_summon`). | |
| 395 | 424 | pub(crate) async fn note_mention( | |
| 396 | 425 | &self, | |
| 397 | 426 | actor: &User, | |
| ⋯ | |||
| 400 | 429 | comment: &Comment, | |
| 401 | 430 | pull_id: Option<&str>, | |
| 402 | 431 | ) -> Result<()> { | |
| 403 | − | if actor.kind == PrincipalKind::Agent | |
| 404 | − | || actor.is_system() | |
| 405 | − | || actor.id == AGENT_ID | |
| 406 | − | || !mentions_agent(&comment.body) | |
| 407 | − | { | |
| 432 | + | if !may_summon(actor) || !mentions_agent(&comment.body) { | |
| 408 | 433 | return Ok(()); | |
| 409 | 434 | } | |
| 410 | 435 | // Whether it may set the agent to work: mentioning spends compute. | |
| ⋯ | |||
| 571 | 596 | return Ok(Outcome::fail(FailureCode::NotFound, "Pull request not found.")); | |
| 572 | 597 | }; | |
| 573 | 598 | let base = pull.base_branch(&repo.default_branch).to_owned(); | |
| 574 | − | // A person asking outranks a stop and the limit on revisions. | |
| 599 | + | // A person asking outranks a stop and the limit on revisions, but | |
| 600 | + | // not this ceiling: each revision spends compute, and nothing | |
| 601 | + | // should send g1t back to one pull request without end. | |
| 602 | + | let now = now_ms(); | |
| 603 | + | let today = self | |
| 604 | + | .db | |
| 605 | + | .prepare("SELECT count(*) AS n FROM agent_mentions WHERE pull_id = ? AND revised_at >= ?") | |
| 606 | + | .bind(&[pull.id.as_str().into(), day_before(now).into()])? | |
| 607 | + | .first::<NumberRow>(None) | |
| 608 | + | .await? | |
| 609 | + | .map_or(0, |row| row.n); | |
| 610 | + | if today >= MAX_MENTION_REVISIONS_PER_DAY { | |
| 611 | + | return Ok(Outcome::fail( | |
| 612 | + | FailureCode::Limit, | |
| 613 | + | format!( | |
| 614 | + | "g1t has been sent back to this pull request {MAX_MENTION_REVISIONS_PER_DAY} times in the last day, the most it takes. Mention it again tomorrow, or push the change yourself." | |
| 615 | + | ), | |
| 616 | + | )); | |
| 617 | + | } | |
| 575 | 618 | let was_stalled = self.is_stalled(&pull.id).await?; | |
| 576 | 619 | self.db | |
| 577 | 620 | .prepare("UPDATE pulls SET stalled = NULL WHERE id = ?") | |
| ⋯ | |||
| 587 | 630 | "g1t is already taking a step on this pull request.", | |
| 588 | 631 | )); | |
| 589 | 632 | } | |
| 633 | + | self.db | |
| 634 | + | .prepare("UPDATE agent_mentions SET revised_at = ? WHERE comment_id = ?") | |
| 635 | + | .bind(&[rfc3339(now).into(), row.comment_id.as_str().into()])? | |
| 636 | + | .run() | |
| 637 | + | .await?; | |
| 590 | 638 | let round = self | |
| 591 | 639 | .db | |
| 592 | 640 | .prepare("SELECT revisions FROM pulls WHERE id = ?") | |
| ⋯ | |||
| 787 | 835 | // the repository is public does not matter here. | |
| 788 | 836 | let target = access::RepoRef { id: &issue.repo_id, namespace: &path.namespace, private: true }; | |
| 789 | 837 | if !actor.verified | |
| 790 | − | || actor.kind == PrincipalKind::Agent | |
| 838 | + | || !may_summon(actor) | |
| 791 | 839 | || !access::can(Some(actor), target, Capability::Run) | |
| 792 | 840 | { | |
| 793 | 841 | return Ok(()); | |
| ⋯ | |||
| 819 | 867 | use super::*; | |
| 820 | 868 | ||
| 821 | 869 | #[test] | |
| 870 | + | fn only_people_and_their_own_tokens_may_summon_g1t() { | |
| 871 | + | let mut ana = User { id: "usr_ana".into(), username: "ana".into(), verified: true, ..User::default() }; | |
| 872 | + | assert!(may_summon(&ana)); | |
| 873 | + | assert!(refuse_job_token::<()>(&ana).is_none()); | |
| 874 | + | ana.token = Some(Box::new(g1t_contracts::scopes::TokenAccess::default())); | |
| 875 | + | assert!(may_summon(&ana), "a person's own token speaks for them"); | |
| 876 | + | ana.token = Some(Box::new(g1t_contracts::scopes::TokenAccess { | |
| 877 | + | job: Some(g1t_contracts::scopes::JobToken { run_id: "run_9".into(), job_id: "job_1".into(), pull_requests: true }), | |
| 878 | + | ..Default::default() | |
| 879 | + | })); | |
| 880 | + | assert!(!may_summon(&ana), "a workflow's comment would set off the workflow again"); | |
| 881 | + | assert!(matches!(refuse_job_token::<()>(&ana), Some(Outcome::Fail(_)))); | |
| 882 | + | let g1t = User { id: AGENT_ID.into(), username: AGENT_NAME.into(), ..User::default() }; | |
| 883 | + | assert!(!may_summon(&g1t)); | |
| 884 | + | } | |
| 885 | + | ||
| 886 | + | #[test] | |
| 887 | + | fn a_day_of_mention_revisions_counts_back_from_now() { | |
| 888 | + | assert_eq!(day_before(2 * 24 * 60 * 60 * 1000), rfc3339(24 * 60 * 60 * 1000)); | |
| 889 | + | assert_eq!(day_before(5), rfc3339(0)); | |
| 890 | + | } | |
| 891 | + | ||
| 892 | + | #[test] | |
| 822 | 893 | fn a_mention_is_found_whatever_its_case() { | |
| 823 | 894 | assert!(mentions_agent("@g1t take this")); | |
| 824 | 895 | assert!(mentions_agent("@G1T take this")); | |
| 379 | 379 | if let Some(refused) = may_plan(&a.actor, &repo) { | |
| 380 | 380 | return Ok(refused); | |
| 381 | 381 | } | |
| 382 | + | if a.assign | |
| 383 | + | && let Some(refused) = crate::mentions::refuse_job_token(&a.actor) | |
| 384 | + | { | |
| 385 | + | return Ok(refused); | |
| 386 | + | } | |
| 382 | 387 | let Some(row) = self | |
| 383 | 388 | .plan_row(&a.id) | |
| 384 | 389 | .await? | |
| ⋯ | |||
| 520 | 525 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 521 | 526 | }; | |
| 522 | 527 | if a.queued { | |
| 528 | + | if let Some(refused) = crate::mentions::refuse_job_token(&a.actor) { | |
| 529 | + | return Ok(refused); | |
| 530 | + | } | |
| 523 | 531 | let repo = match self.repo(&a.repo, &Some(a.actor.clone())).await? { | |
| 524 | 532 | Outcome::Ok(repo) => repo, | |
| 525 | 533 | Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)), | |
| 17 | 17 | ||
| 18 | 18 | const MAX_REQUIRED_APPROVALS: u32 = 6; | |
| 19 | 19 | const MAX_REVISIONS: u32 = 5; | |
| 20 | + | /// The most times in a day people's mentions may send g1t back to one pull | |
| 21 | + | /// request. They outrank `MAX_REVISIONS` and a stall, but not this. | |
| 22 | + | pub(crate) const MAX_MENTION_REVISIONS_PER_DAY: u32 = 10; | |
| 20 | 23 | ||
| 21 | 24 | #[derive(Deserialize)] | |
| 22 | 25 | struct SettingsRow { |
| 24 | 24 | { "binding": "ACTIONS", "service": "g1t-actions" } | |
| 25 | 25 | ], | |
| 26 | 26 | "queues": { | |
| 27 | − | "consumers": [{ "queue": "g1t-events-work", "max_batch_size": 100, "max_batch_timeout": 1 }] | |
| 27 | + | "consumers": [{ "queue": "g1t-events-work", "max_batch_size": 100, "max_batch_timeout": 1, "max_retries": 3, "dead_letter_queue": "g1t-events-dlq" }] | |
| 28 | 28 | }, | |
| 29 | 29 | "observability": { "enabled": true } | |
| 30 | 30 | } |