diff --git a/CONTEXT.md b/CONTEXT.md index b3d663f53..a4b9d63e3 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -47,8 +47,12 @@ A finding from the Standards axis of a code review: the change breaks a document **Spec finding**: A finding from the Spec axis of a code review: the change is missing, gets wrong, or goes beyond what the issue asked for. +**Security finding**: +A vulnerability a **Security review** or a **Security audit** found. One the **Run**'s own change introduced, left unaddressed, holds a **Self-merge** into the **Base branch**. One already on the Base branch is never written up in public before it is fixed: it is recorded privately for the **Day shift**, as a draft security advisory on a public repository or a labelled issue on a private one. Once recorded, it waits for the Day shift, unless it is reproduced and fixing is allowed, until the Day shift triages it: by closing its draft advisory, publishing it or giving it a severity, or by taking `needs-triage` off its issue. That decision is also its grade, real or not. +_Avoid_: vulnerability report, security issue + **Unaddressed finding**: -A **Standards finding** or **Spec finding** the agent chose not to fix, listed in the pull request body with what refutes it: a line of code, a plan decision, an ADR or a run that exercises the finding. When a Spec or the Day shift must decide, the entry instead links the `needs-triage` issue filed for that decision, or the open issue already covering it. +A **Standards finding** or **Spec finding** the agent chose not to fix, listed in the pull request body with what refutes it: a line of code, a plan decision, an ADR or a run that exercises the finding. When a Spec or the Day shift must decide, the entry instead links the `needs-triage` issue filed for that decision, or the open issue already covering it. A **Security finding** the change itself introduced, which the **Security review** left unfixed, is listed there too, and holds a **Self-merge** into the **Base branch**. **Spec**: An issue describing a multi-session piece of work, what is being built rather than how each session does its share, made of **Tickets**: its GitHub sub-issues. See [Spec](https://www.aihero.dev/ai-coding-dictionary/spec) in Matt Pocock's AI Coding Dictionary. @@ -59,13 +63,13 @@ An issue scoping one session of work, standing alone or as a sub-issue of a **Sp _Avoid_: task, sub-task **Run**: -One invocation of the factory on a single issue that is not a **Spec**, from launch to cleanup, ending in one pull request. Started directly on an **Issue URL**, by a **Spec run** for one of its **Tickets**, by an **Architect run** for the Ticket its **Architecture review** published, or by a **Pickup run** for the **Ready issue** it took. +One invocation of the factory on a single issue that is not a **Spec**, from launch to cleanup, ending in one pull request. Started directly on an **Issue URL**, by a **Spec run** for one of its **Tickets**, by an **Architect run** for the Ticket its **Architecture review** published, by a **Pickup run** for the **Ready issue** it took, or by a **Security run** for a **Security finding** it fixes. **Worktree**: A factory-owned checkout, on an **Issue branch** for a **Run** or detached for an **Architecture review**. Acquisition accounts for partial local effects, removing only attempt-owned clean artifacts on failure and retaining and naming work or uncertain artifacts. Failed-acquisition retention survives later Architecture review attempts. Successful acquisition carries checkout and registration identity through its lifetime; ordinary synchronization and observations act only on that acquired checkout and selected Issue branch, refusing changed or uncertain ownership. Confirmed Self-merge branch deletion uses the captured Launch repository independently of checkout validity. Failed run preservation acts only on that acquired instance and its Issue branch, retaining work when ownership has changed or is uncertain. Cleanup removes only that acquired instance, retaining and naming replaced or uncertain resources. Stale Architecture review scratch is disposable only with evidence of successful detached acquisition and unchanged ownership; unmarked or uncertain checkouts are retained. **Claim**: -The mark that the factory has taken an issue: thirdshift labels the issue `in-progress`, in place of `ready-for-agent` if it has that, when a **Run** or a **Spec run** starts on it, whether started on its **Issue URL** or by an **Architect run** or a **Pickup run**. A **Ticket**'s Run in a Spec run and a **Base fix** make none. Ending a Claim finishes even after Command interruption. A Claim is released, `ready-for-agent` put back, only when the Run ends with nothing on `origin` to take over: no **Issue branch** and no pull request. Otherwise it stays while the issue is open, whether its pull request is ready for review or the Run failed, until the **Day shift** relabels it, and thirdshift takes the label off once the issue is closed. +The mark that the factory has taken an issue: thirdshift labels the issue `in-progress`, in place of `ready-for-agent` if it has that, when a **Run** or a **Spec run** starts on it, whether started on its **Issue URL** or by an **Architect run**, a **Pickup run** or a **Security run**. A **Ticket**'s Run in a Spec run and a **Base fix** make none. Ending a Claim finishes even after Command interruption. A Claim is released, `ready-for-agent` put back, only when the Run ends with nothing on `origin` to take over: no **Issue branch** and no pull request. Otherwise it stays while the issue is open, whether its pull request is ready for review or the Run failed, until the **Day shift** relabels it, and thirdshift takes the label off once the issue is closed. _Avoid_: lock (a lock is per machine and ends with its process), assignment **Spec run**: @@ -73,19 +77,27 @@ One invocation of the factory on a **Spec**'s **Issue URL**: it works through th _Avoid_: batch run, spec implementation **Architect run**: -One invocation of the factory with no **Issue URL**: an **Architecture review** of the **Base branch**, then, unless asked to stop there, a **Spec run** or a **Run** on the **Architect plan** it published. One pass per invocation. It is skipped, doing nothing, when another Architect run or a **Pickup run** on the same repository is still running on the machine, its Spec run or Run included, when an earlier Architect plan is still open, when an **Architect idea** is waiting for triage, or when the repository has a **Ready issue**, so that work a human shaped goes first. Skipped is not a failure, and sends no **Run notification**, so that it can be started every few minutes and so follow the last one as soon as its plan is merged. +One invocation of the factory with no **Issue URL**: an **Architecture review** of the **Base branch**, then, unless asked to stop there, a **Spec run** or a **Run** on the **Architect plan** it published. One pass per invocation. It is skipped, doing nothing, when another **Pass** on the same repository is still running on the machine, its Spec run or Run included, when an earlier Architect plan is still open, when an **Architect idea** is waiting for triage, or when the repository has a **Ready issue**, so that work a human shaped goes first. Skipped is not a failure, and sends no **Run notification**, so that it can be started every few minutes and so follow the last one as soon as its plan is merged. _Avoid_: improve run, architecture run **Weeding**: The factory clearing the detritus that features leave in a repository's codebase as they land, with no human starting it: **Architect runs** started often enough that each follows the last as soon as its **Architect plan** is merged. It yields to work a human shaped, and pauses for the **Day shift** whenever a plan fails or an **Architect idea** waits for triage. _Avoid_: upkeep, continuous architect, daemon, loop +**Fencing**: +The factory finding and fixing vulnerabilities in a repository's codebase with no human starting it: **Security runs** started as often as the operating system's scheduler is set to start them, with fixing allowed. Like **Weeding**, it yields to work a human shaped, and pauses for the **Day shift** whenever a fix fails or a **Security finding** waits for triage. +_Avoid_: patrol, continuous security, security loop + **Pickup run**: -One invocation of the factory with no **Issue URL**: it takes the lowest-numbered **Ready issue** in the repository and starts a **Spec run** or a **Run** on it, as the command on that issue's URL would. One issue per invocation, and thirdshift never schedules itself: the operating system's scheduler starts each one. It is skipped, doing nothing, when another Pickup run or an **Architect run** on the same repository is still running on the machine, its Spec run or Run included, when there is no Ready issue, or when the repository is at its **Claim limit**. Skipped is not a failure, and sends no **Run notification**. +One invocation of the factory with no **Issue URL**: it takes the lowest-numbered **Ready issue** in the repository and starts a **Spec run** or a **Run** on it, as the command on that issue's URL would. One issue per invocation, and thirdshift never schedules itself: the operating system's scheduler starts each one. It is skipped, doing nothing, when another **Pass** on the same repository is still running on the machine, its Spec run or Run included, when there is no Ready issue, or when the repository is at its **Claim limit**. Skipped is not a failure, and sends no **Run notification**. _Avoid_: watch, daemon, queue run, poll +**Security run**: +One invocation of the factory with no **Issue URL**: a **Security audit** of the **Base branch**, then, for each **Security finding**, a session that tries to reproduce it with a proof-of-concept test in a throwaway worktree, and scores the severity of each one it reproduces. When a flag or the **User config** allows it, each reproduced finding is fixed by a **Run**, or by a **Spec run** when one session isn't enough, on issues that say only what the fix changes. Every other finding waits for the **Day shift**, and one nothing reproduces is never fixed unattended. It is skipped, like an **Architect run**, when another **Pass** on the same repository is still running on the machine, when the repository has a **Ready issue**, when one of its findings still waits for the Day shift, when a fix it dispatched failed and is still open, or when the Base branch hasn't changed since the last Security audit that finished. A failed fix is left for the Day shift; no later Security run or **Pickup run** retries it. +_Avoid_: Secure run (it reads as a run that is secure), audit run, security pass + **Pass**: -An **Architect run** or a **Pickup run**: a **Command** started with no **Issue URL**, which decides before any work whether it is skipped. +An **Architect run**, a **Pickup run** or a **Security run**: a **Command** started with no **Issue URL**, which decides before any work whether it is skipped. _Avoid_: tick, cycle **Claim limit**: @@ -112,6 +124,10 @@ _Avoid_: plan, plan doc The issue an **Architecture review** ends on when its top recommendation is not Strong: the one it filed, or the open issue that already covered it. thirdshift labels it `architect-idea` and `needs-triage`, the second put back on an issue that already covered the idea and had been triaged, since the factory again takes it for the best next move. While any Architect idea is open and still labelled `needs-triage`, every **Architect run** on the repository is skipped: the factory has run out of Strong ideas, and waits for the **Day shift** to triage one, whatever the decision. _Avoid_: weak plan, suggestion +**Security audit**: +The agent session that opens a **Security run**: it audits the **Base branch** for vulnerabilities, and records each **Security finding** privately for the **Day shift**. It changes nothing in the repository, and writes nothing public about a vulnerability. +_Avoid_: security scan, security review (a **Security review** reviews one Run's change) + **Spec branch**: The **Issue branch** of the **Spec** in a **Spec run**, branched off the **Base branch**. Each **Ticket**'s **Run** is a **Merge run** into it, so the Spec's work gathers there before it reaches the Base branch. _Avoid_: integration branch, feature branch @@ -128,6 +144,10 @@ An open **Ticket** labelled with a triage role that says it is not agent work: ` **Spec review**: The agent session a **Spec run** starts once every **Ticket** is done: it reviews the whole **Spec branch** against the **Base branch** and the **Spec**, fixes what it agrees with, and writes the **Spec PR**'s description, listing the rest as **Unaddressed findings**. +**Security review**: +When a flag or the **User config** asks for one, the agent session a **Run** starts after its opening session, or a **Spec run** starts on its **Spec PR** after the **Spec review**: it reviews the branch's change against the **Base branch** for vulnerabilities, and fixes each **Security finding** it can show with a failing test. A **Ticket**'s Run gets none. A Security review that a model refused, or that ended early, holds a **Self-merge** as an unaddressed Security finding does. +_Avoid_: security axis, security audit (a **Security audit** covers the whole Base branch) + **Failed spec run**: A **Spec run** that ends, including by interruption, with any of its **Tickets** not done, or with its **Spec PR** not ready, mergeable and green (or, when asked to merge, not merged). It still takes every Ticket it can reach, and a failed Ticket stops only the Tickets it blocks. After Command interruption it starts no new Ticket or Spec review, drains its active Tickets, and finishes publishing their final Tickets checklist and outcome lines. @@ -136,7 +156,7 @@ A **Run** asked to end with its pull request merged rather than left for review, _Avoid_: auto-merge (GitHub's own feature, which thirdshift does not use) **Run notification**: -A message thirdshift sends when a **Run**, a **Spec run** or an **Architect run** ends, whatever its outcome (ready, merged, failed or interrupted; for an Architect run that dispatched nothing, plan published, idea filed, idea already filed or review failed), to the address given with the email flag or the default in the **User config**. Each sends one only when asked to, by the flag or by the User config; a Spec run's notification lists each **Ticket**'s outcome, and a Ticket's **Run** never sends one of its own. After an **Inherited failure** it carries, beside the cause, what the Run says on stderr: where the checks fail on the **Base branch**, and the offer of a **Base fix** if nobody decided against one. An Architect run's notification tells how its **Architecture review** ended, naming the plan or idea issue, and how the Spec run or Run it dispatched ended, which sends none of its own; a skipped one sends none. A **Pickup run** that took a **Ready issue** sends the one the Spec run or Run it dispatched would have sent, which sends none of its own; a skipped one sends none. Failing to send one never changes the outcome. +A message thirdshift sends when a **Run**, a **Spec run**, an **Architect run** or a **Security run** ends, whatever its outcome (ready, merged, failed or interrupted; for an Architect run that dispatched nothing, plan published, idea filed, idea already filed or review failed), to the address given with the email flag or the default in the **User config**. Each sends one only when asked to, by the flag or by the User config; a Spec run's notification lists each **Ticket**'s outcome, and a Ticket's **Run** never sends one of its own. After an **Inherited failure** it carries, beside the cause, what the Run says on stderr: where the checks fail on the **Base branch**, and the offer of a **Base fix** if nobody decided against one. An Architect run's notification tells how its **Architecture review** ended, naming the plan or idea issue, and how the Spec run or Run it dispatched ended, which sends none of its own; a skipped one sends none. A **Pickup run** that took a **Ready issue** sends the one the Spec run or Run it dispatched would have sent, which sends none of its own; a skipped one sends none. A **Security run**'s notification lists each **Security finding**'s severity, title and private link, never its write-up, since the message passes through a third party; a skipped one sends none. Failing to send one never changes the outcome. _Avoid_: completion email, alert **User config**: @@ -200,13 +220,13 @@ The full transcript of one agent session, one per session, a **Resume** and each _Avoid_: transcript, session file **Command**: -One invocation of thirdshift that does factory work: a **Run** or **Spec run** started on an **Issue URL**, an **Architect run**, or a **Pickup run**. A child Run, a **Ticket**'s Run or a **Base fix**, is part of the command that started it, not one of its own. Interruption stops its ordinary CLI work and prevents more from starting, while Failed run salvage, confirmed Self-merge completion, Claim ending and Spec accounting still finish; finishing never clears the recorded interruption. +One invocation of thirdshift that does factory work: a **Run** or **Spec run** started on an **Issue URL**, an **Architect run**, a **Pickup run**, or a **Security run**. A child Run, a **Ticket**'s Run or a **Base fix**, is part of the command that started it, not one of its own. Interruption stops its ordinary CLI work and prevents more from starting, while Failed run salvage, confirmed Self-merge completion, Claim ending and Spec accounting still finish; finishing never clears the recorded interruption. _Avoid_: invocation, job **Command log**: -Everything one **Run**, **Spec run**, **Architect run** or **Pickup run** printed, kept in a file of its own with its repository's other logs, stamped like its **Session logs**. An Architect run's or Pickup run's covers the Spec run or Run it dispatched, and a Spec run's covers its **Tickets**' Runs, as a Run's covers its **Base fix**: none of those keeps one of its own. It is kept once the command starts work: a Pickup run or Architect run skipped before doing any work keeps none, and neither does a Run that fails before it starts work, such as on **Origin match**. +Everything one **Run**, **Spec run**, **Architect run**, **Pickup run** or **Security run** printed, kept in a file of its own with its repository's other logs, stamped like its **Session logs**. An Architect run's, Pickup run's or Security run's covers the Spec runs or Runs it dispatched, and a Spec run's covers its **Tickets**' Runs, as a Run's covers its **Base fix**: none of those keeps one of its own. It is kept once the command starts work: a Pickup run, Architect run or Security run skipped before doing any work keeps none, and neither does a Run that fails before it starts work, such as on **Origin match**. _Avoid_: cron log, output log, run log (a **Run** is one kind of command) **Activity log**: -One running record per repository of what the factory did there: a line when a **Run**, **Spec run**, **Architect run** or **Pickup run** starts work, naming its **Command log**, and a line when it ends, with its outcome. A skipped Architect run or Pickup run writes a line only when its reason differs from the last one recorded for its kind, so a repository that sits idle shows one line, not one per pass. It is what thirdshift writes about a scheduled pass, in place of output to the scheduler. +One running record per repository of what the factory did there: a line when a **Run**, **Spec run**, **Architect run**, **Pickup run** or **Security run** starts work, naming its **Command log**, and a line when it ends, with its outcome. A skipped Architect run, Pickup run or Security run writes a line only when its reason differs from the last one recorded for its kind, so a repository that sits idle shows one line, not one per pass. It is what thirdshift writes about a scheduled pass, in place of output to the scheduler. _Avoid_: cron log, pass log, history diff --git a/docs/adr/0015-security-findings-stay-private-until-fixed.md b/docs/adr/0015-security-findings-stay-private-until-fixed.md new file mode 100644 index 000000000..a9c4de55c --- /dev/null +++ b/docs/adr/0015-security-findings-stay-private-until-fixed.md @@ -0,0 +1,17 @@ +# A Security finding stays private until it is fixed, and a Security run may fix a reproduced one in public + +A public write-up of a vulnerability already on a public repository's **Base branch** discloses it, and a model can turn even a public fix into a working exploit within hours ([security-agent-passes.md](../research/security-agent-passes.md) §7.4). So a **Security run** never files a **Security finding** as an ordinary issue. It records the finding privately: a draft repository security advisory on a public repository, or an issue labelled `security-finding` on a private one. The **Run notification** names the finding without its write-up. GitHub's own private fix path can't run unattended: no CI runs in an advisory's temporary private fork, and only a button in the web UI merges it (§5.2). So when a flag or the **User config** allows it, the Security run fixes a finding that a proof-of-concept test has reproduced through an ordinary **Run** or **Spec run**. It works from terse public issues that link the private record and say only what the fix changes. The fix discloses the vulnerability when it is pushed. That window is the price of fixing without a human, and a finding nothing reproduces is never fixed unattended. + +## Considered Options + +- **Findings as ordinary issues, the way an Architecture review publishes an Architect plan.** Rejected. On a public repository the issue would publish an unfixed vulnerability, and a failed fix would leave that write-up public until the **Day shift** acted. +- **GitHub's temporary private fork for every fix.** Rejected as the factory's path, since CI can't run there and only a human can merge it. The Day shift can still use it by hand. +- **Report only, never fix unattended.** Kept as the default: fixing is off until a flag or the User config allows it, as a **Base fix** is. +- **Hold every fix for the Day shift's review.** Rejected as the only mode. A fix waiting in a public pull request discloses the vulnerability for as long as it waits, and on a repository that deploys on merge, a quick **Merge run** is the shortest exposure. + +## Consequences + +- The Day shift triages the private record, not an issue, and its triage ends a finding's wait: closing the draft advisory, publishing it or giving it a severity, or taking `needs-triage` off the private issue. +- GitHub keeps repository security advisories for public repositories, so a private repository's finding is an issue, private along with its repository. +- A Run notification passes through Resend, a third party, so it carries a finding's severity, title and link, never its write-up. +- A **Security review** that meets a vulnerability already on the Base branch sends it to the same private record, out of its Run's pull request.