thirdshift-to-tickets
- Used by the Architecture review, in an Architect run
+ Used by the Architecture review, in an Architect run, or Security fix publishing, in a Security run
thirdshift-to-tickets/SKILL.md
---
name: thirdshift-to-tickets
-description: Break a plan, spec, or the current session into a set of tracer-bullet tickets, each declaring its blocking edges, published to the issue tracker with native sub-issue and blocking links. Only for an Architecture review.
+description: Break a plan, spec, or the current session into a set of tracer-bullet tickets, each declaring its blocking edges, published to the issue tracker with native sub-issue and blocking links. For an Architecture review or Security fix publishing.
disable-model-invocation: false
---
@@ -6784,6 +6787,8 @@ thirdshift-to-tickets/SKILL.m
The tickets are all this skill writes. Write nothing to the repository: no ticket files, commits or branches. A `CONTEXT.md` or ADR change the work needs is part of a ticket's work, named in its acceptance criteria.
+For **Security fix publishing**, keep every Ticket terse: say only what the fix changes and link the private record. Keep all finding evidence in that record. A private finding's issue is the parent Spec, and its existing body and evidence stay unchanged.
+
## Process
### 1. Gather context
diff --git a/skills/thirdshift-to-spec/SKILL.md b/skills/thirdshift-to-spec/SKILL.md
index 56dc42ed..5990f1a8 100644
--- a/skills/thirdshift-to-spec/SKILL.md
+++ b/skills/thirdshift-to-spec/SKILL.md
@@ -1,6 +1,6 @@
---
name: thirdshift-to-spec
-description: "Turn the current session into a spec and publish it to the project issue tracker: no interview, just synthesis of what you've already settled. Only for an Architecture review."
+description: "Turn the current session into a spec and publish it to the project issue tracker: no interview, just synthesis of what you've already settled. For an Architecture review or Security fix publishing."
disable-model-invocation: false
---
@@ -10,6 +10,8 @@ The issue tracker and triage label vocabulary should have been provided to you.
The spec is all this skill writes. Write nothing to the repository: no spec file, commits or branches. A `CONTEXT.md` or ADR change the spec needs is work for one of its tickets.
+For **Security fix publishing**, the Session prompt overrides the template: keep the Spec terse, saying only what the fix changes and linking the private record. Keep all finding evidence in that record. On a private repository, use the finding's existing issue as the Spec and preserve its body and evidence; publish its Tickets with `thirdshift-to-tickets`.
+
## Process
1. Explore the repo to understand the current state of the codebase, if you haven't already. Use the project's domain glossary vocabulary throughout the spec, and respect any ADRs in the area you're touching.
diff --git a/skills/thirdshift-to-tickets/SKILL.md b/skills/thirdshift-to-tickets/SKILL.md
index 2ed473f3..d9568644 100644
--- a/skills/thirdshift-to-tickets/SKILL.md
+++ b/skills/thirdshift-to-tickets/SKILL.md
@@ -1,6 +1,6 @@
---
name: thirdshift-to-tickets
-description: Break a plan, spec, or the current session into a set of tracer-bullet tickets, each declaring its blocking edges, published to the issue tracker with native sub-issue and blocking links. Only for an Architecture review.
+description: Break a plan, spec, or the current session into a set of tracer-bullet tickets, each declaring its blocking edges, published to the issue tracker with native sub-issue and blocking links. For an Architecture review or Security fix publishing.
disable-model-invocation: false
---
@@ -12,6 +12,8 @@ The issue tracker and triage label vocabulary should have been provided to you.
The tickets are all this skill writes. Write nothing to the repository: no ticket files, commits or branches. A `CONTEXT.md` or ADR change the work needs is part of a ticket's work, named in its acceptance criteria.
+For **Security fix publishing**, keep every Ticket terse: say only what the fix changes and link the private record. Keep all finding evidence in that record. A private finding's issue is the parent Spec, and its existing body and evidence stay unchanged.
+
## Process
### 1. Gather context
diff --git a/src/github/advisories.rs b/src/github/advisories.rs
index 44a954af..636e0abd 100644
--- a/src/github/advisories.rs
+++ b/src/github/advisories.rs
@@ -8,7 +8,7 @@ use serde_json::{Value, json};
use super::GitHub;
use crate::issue::IssueUrl;
use crate::labels::{Label, NEEDS_TRIAGE};
-use crate::security::reproduction::{Reproduction, Severity};
+use crate::security::reproduction::{FixSize, Reproduction, Severity};
const SECURITY_FINDING: Label = Label::new(
"security-finding",
@@ -69,7 +69,9 @@ impl SecurityRecords {
{
continue;
}
- if let Some(severity) = reproduced_severity(&value[description]) {
+ if let Some((severity, _)) =
+ reproduced_outcome(value[description].as_str().unwrap_or_default())
+ {
// The Day shift's current advisory grade takes precedence
// over the historical proof-of-concept's score.
let severity = value["severity"]
@@ -148,14 +150,18 @@ impl SecurityRecords {
}
fn reproduced_severity(description: &Value) -> Option {
- let (_, reproduction) = description
- .as_str()?
- .split_once("\n\n")?;
+ reproduced_outcome(description.as_str()?).map(|(severity, _)| severity)
+}
+
+fn reproduced_outcome(description: &str) -> Option<(Severity, FixSize)> {
+ let (_, reproduction) =
+ description.split_once("\n\n")?;
let outcome = reproduction
.lines()
.find_map(|line| line.strip_prefix("Outcome: "))?;
match outcome.split_whitespace().collect::>().as_slice() {
- ["reproduced", severity, "single" | "spec"] => Severity::parse(severity),
+ ["reproduced", severity, "single"] => Some((Severity::parse(severity)?, FixSize::Single)),
+ ["reproduced", severity, "spec"] => Some((Severity::parse(severity)?, FixSize::Spec)),
_ => None,
}
}
@@ -181,6 +187,13 @@ pub enum SecurityRecord {
}
impl SecurityRecord {
+ /// The size judged by the completed reproduction, never the candidate write-up.
+ pub fn fix_size(&self) -> Result {
+ reproduced_outcome(self.description())
+ .map(|(_, size)| size)
+ .context("the Security finding has no reproduced fix size")
+ }
+
/// A Day-shift decision is the finding's grade; a repeated fingerprint
/// must not publish new proof-of-concept evidence or replace that grade.
pub fn untriaged(&self) -> bool {
@@ -218,12 +231,12 @@ pub struct DraftAdvisory {
}
impl GitHub {
- /// Link the dispatched Ticket without changing the private record's grade.
+ /// Link the dispatched fix issue without changing the private record's grade.
pub fn link_security_fix(
&self,
repo: &str,
record: &SecurityRecord,
- ticket: &IssueUrl,
+ issue: &IssueUrl,
) -> Result<()> {
let (path, field) = match record {
SecurityRecord::Advisory { id, .. } => (
@@ -249,7 +262,7 @@ impl GitHub {
let description = format!(
"{}{FIX_TICKET_MARKER}{}\n",
record.description().trim_end(),
- ticket.url
+ issue.url
);
let body = serde_json::to_vec(&json!({field: description}))?;
let output = self.output_with_input(
@@ -265,8 +278,8 @@ impl GitHub {
Ok(())
}
- pub fn security_fix_text(&self, ticket: &IssueUrl) -> Result {
- let value = self.issue_view(ticket, "title,body")?;
+ pub fn security_fix_text(&self, issue: &IssueUrl) -> Result {
+ let value = self.issue_view(issue, "title,body")?;
let title = value["title"]
.as_str()
.context("the Security fix Ticket has no title")?;
diff --git a/src/pass.rs b/src/pass.rs
index 21732ee2..f86f57b9 100644
--- a/src/pass.rs
+++ b/src/pass.rs
@@ -43,14 +43,12 @@ const REVIEW: &str = "architecture-review";
/// out.
pub enum Dispatch<'a> {
SecurityFix {
- ticket: &'a IssueUrl,
+ issue: &'a IssueUrl,
+ is_spec: bool,
base: &'a str,
},
/// An Architect run's Architect plan.
- ArchitectPlan {
- plan: &'a IssueUrl,
- base: &'a str,
- },
+ ArchitectPlan { plan: &'a IssueUrl, base: &'a str },
/// The Ready issue a Pickup run took, a Spec or not, as `is_spec` says.
ReadyIssue {
issue: &'a IssueUrl,
@@ -136,7 +134,7 @@ pub trait Outside {
record: &SecurityRecord,
url: &str,
) -> (Result, Option);
- fn link_security_fix(&mut self, record: &SecurityRecord, ticket: &IssueUrl) -> Result<()>;
+ fn link_security_fix(&mut self, record: &SecurityRecord, issue: &IssueUrl) -> Result<()>;
/// Run `dispatch` to its end, on the Harness the pass checked.
fn dispatch(&mut self, dispatch: Dispatch) -> Ended;
}
@@ -279,17 +277,12 @@ impl Outside for LaunchAndGitHub<'_> {
/// the command's flags and the User config, on the checked Harness.
fn dispatch(&mut self, dispatch: Dispatch) -> Ended {
let (issue, asks, base) = match dispatch {
- Dispatch::SecurityFix { ticket, base } => (
- ticket,
- Asks::of_ready_issue(ticket, false, self.flags, self.config),
- base,
- ),
- Dispatch::ArchitectPlan { plan, base } => (
- plan,
- Asks::of_architect_plan(plan, self.flags, self.config),
+ Dispatch::SecurityFix {
+ issue,
+ is_spec,
base,
- ),
- Dispatch::ReadyIssue {
+ }
+ | Dispatch::ReadyIssue {
issue,
is_spec,
base,
@@ -298,6 +291,11 @@ impl Outside for LaunchAndGitHub<'_> {
Asks::of_ready_issue(issue, is_spec, self.flags, self.config),
base,
),
+ Dispatch::ArchitectPlan { plan, base } => (
+ plan,
+ Asks::of_architect_plan(plan, self.flags, self.config),
+ base,
+ ),
};
let mut asks = Asks {
harness: self.harness.clone(),
@@ -322,8 +320,8 @@ impl Outside for LaunchAndGitHub<'_> {
)
}
- fn link_security_fix(&mut self, record: &SecurityRecord, ticket: &IssueUrl) -> Result<()> {
- GitHub::new().link_security_fix(&self.repo.slug(), record, ticket)
+ fn link_security_fix(&mut self, record: &SecurityRecord, issue: &IssueUrl) -> Result<()> {
+ GitHub::new().link_security_fix(&self.repo.slug(), record, issue)
}
}
@@ -390,7 +388,8 @@ mod in_memory {
PublishSecurityFix(String),
LinkSecurityFix(u64),
DispatchSecurityFix {
- ticket: u64,
+ issue: u64,
+ is_spec: bool,
base: String,
},
/// It recorded that it started work: on this issue, for a Pickup
@@ -798,8 +797,13 @@ mod in_memory {
fn dispatch(&mut self, dispatch: Dispatch) -> Ended {
self.calls.push(match dispatch {
- Dispatch::SecurityFix { ticket, base } => Call::DispatchSecurityFix {
- ticket: ticket.number,
+ Dispatch::SecurityFix {
+ issue,
+ is_spec,
+ base,
+ } => Call::DispatchSecurityFix {
+ issue: issue.number,
+ is_spec,
base: base.into(),
},
Dispatch::ArchitectPlan { plan, base } => Call::DispatchPlan {
@@ -834,8 +838,8 @@ mod in_memory {
)
}
- fn link_security_fix(&mut self, _record: &SecurityRecord, ticket: &IssueUrl) -> Result<()> {
- self.calls.push(Call::LinkSecurityFix(ticket.number));
+ fn link_security_fix(&mut self, _record: &SecurityRecord, issue: &IssueUrl) -> Result<()> {
+ self.calls.push(Call::LinkSecurityFix(issue.number));
Ok(())
}
}
diff --git a/src/prompt.rs b/src/prompt.rs
index e9cd5838..ee02c71b 100644
--- a/src/prompt.rs
+++ b/src/prompt.rs
@@ -330,16 +330,18 @@ pub fn security_reproduction(commit: &str, finding: &str, test: &std::path::Path
}
pub const SECURITY_FIX_LINE: &str = "Security fix Ticket: ";
+pub const SECURITY_FIX_SPEC_LINE: &str = "Security fix Spec: ";
pub fn security_fix(base: &str, url: &str, finding: &str) -> String {
format!(
"Publish the fix for this reproduced Security finding on Base branch `{base}`.\n\
Read the private record at {url} and the record below.\n\
- Publish exactly one new, terse Ticket in this repository, labelled `needs-triage`. Create that label if missing.\n\
- The Ticket says only what the fix changes and links the private record. It carries none of the write-up, trace, evidence, reproduction notes, proof-of-concept test or exploit details, even paraphrased. Keep those in the private record.\n\
- For now every fix is a single Ticket, even if the reproduction suggested a Spec. Publish no Spec or sub-issues.\n\
- This session changes no repository source, commits and pushes nothing, opens no pull request, and does not implement the fix. thirdshift checks the Ticket, marks it ready, labels it security-fix and dispatches its Run.\n\
- End your final message with exactly `{SECURITY_FIX_LINE}`.\n\n\
+ Follow the completed reproduction's fix size: single publishes one Ticket; spec publishes a Spec with Tickets using `thirdshift-to-spec` and `thirdshift-to-tickets`.\n\
+ On a public repository, publish one new top issue labelled `needs-triage`. Create that label if missing. On a private repository, reuse the finding's issue as the top issue and preserve its body and evidence; add the bigger fix's Tickets as its native sub-issues.\n\
+ Every new issue is terse: it says only what the fix changes and links the private record. It carries none of the write-up, trace, evidence, reproduction notes, proof-of-concept test or exploit details, even paraphrased. Keep those in the private record. This overrides the skills' templates.\n\
+ A Spec's Tickets must be new, open, labelled `ready-for-agent`, linked as native sub-issues with their native blocking links, and have no sub-issues of their own. Read the links back before finishing.\n\
+ This session changes no repository source, commits and pushes nothing, opens no pull request, and does not implement the fix. thirdshift checks every issue, marks the top issue ready, labels all fix issues security-fix and dispatches its Run or Spec run.\n\
+ End your final message with exactly `{SECURITY_FIX_LINE}` for single or `{SECURITY_FIX_SPEC_LINE}` for spec, naming the top issue.\n\n\
Private record:\n{finding}\n\n{HEADLESS}"
)
}
diff --git a/src/prompts_page.rs b/src/prompts_page.rs
index 736b69d8..eb95a089 100644
--- a/src/prompts_page.rs
+++ b/src/prompts_page.rs
@@ -198,7 +198,7 @@ fn prompts() -> Vec {
Prompt {
id: "prompt-security-fix",
title: "Security fix publishing",
- when: "With fixing allowed, publishes one terse Ticket for the most severe reproduced finding. thirdshift checks and marks it ready before dispatching its Run.",
+ when: "With fixing allowed, publishes a terse Ticket or Spec with Tickets for the most severe reproduced finding, reusing a private finding's issue. thirdshift checks and marks it ready before dispatching its Run or Spec run.",
sender: SecurityRun,
text: prompt::security_fix(BASE, "", ""),
},
@@ -367,12 +367,12 @@ fn skill_used_by(skill: &str) -> String {
"thirdshift-resolving-merge-conflicts" => {
format!("{}, in a Repair", unit_links(&[FINISH]))
}
- "thirdshift-improve-codebase-architecture"
- | "thirdshift-to-spec"
- | "thirdshift-to-tickets"
- | "thirdshift-codebase-design" => {
+ "thirdshift-improve-codebase-architecture" | "thirdshift-codebase-design" => {
"the Architecture review, in an Architect run".to_string()
}
+ "thirdshift-to-spec" | "thirdshift-to-tickets" => {
+ "the Architecture review, in an Architect run, or Security fix publishing, in a Security run".to_string()
+ }
"thirdshift-security-audit" => {
"the Security audit, in a Security run, or a Security review".to_string()
}
@@ -657,6 +657,23 @@ mod tests {
);
}
+ #[test]
+ fn publishing_skills_show_security_fix_publishing_as_a_user() {
+ let html = render();
+ for name in ["thirdshift-to-spec", "thirdshift-to-tickets"] {
+ let (_, card) = html
+ .split_once(&format!("id=\"skill-{name}\""))
+ .expect("the page lists the publishing skill");
+ let (card, _) = card.split_once("
").unwrap();
+ let (_, usage) = card
+ .split_once("Used by ")
+ .expect("the card names its users");
+ let (usage, _) = usage.split_once("").unwrap();
+ assert!(usage.contains("Security fix publishing"), "{name}: {usage}");
+ assert!(usage.contains("Architecture review"), "{name}: {usage}");
+ }
+ }
+
#[test]
fn prompts_page_shows_cloudflares_security_audit_and_its_mit_license() {
let html = render();
diff --git a/src/security.rs b/src/security.rs
index 5e1a1ee3..62fc642e 100644
--- a/src/security.rs
+++ b/src/security.rs
@@ -224,10 +224,11 @@ fn fix(
outside.started(Work::SecurityRun(repo));
let (published, session_log) = outside.publish_security_fix(base, &record, &metadata.url);
log = session_log;
- let ticket = published?;
- outside.link_security_fix(&record, &ticket)?;
+ let issue = published?;
+ outside.link_security_fix(&record, &issue)?;
Ok(outside.dispatch(crate::pass::Dispatch::SecurityFix {
- ticket: &ticket,
+ issue: &issue,
+ is_spec: record.fix_size()? == reproduction::FixSize::Spec,
base,
}))
})();
@@ -366,7 +367,8 @@ mod tests {
.contains(&Call::PublishSecurityFix("GHSA-first".into()))
);
assert!(outside.calls.contains(&Call::DispatchSecurityFix {
- ticket: 8,
+ issue: 8,
+ is_spec: false,
base: "main".into()
}));
assert!(!outside.calls.contains(&Call::AuditHistory));
diff --git a/src/security/fixing.rs b/src/security/fixing.rs
index 8318e073..a0f8940a 100644
--- a/src/security/fixing.rs
+++ b/src/security/fixing.rs
@@ -1,4 +1,4 @@
-//! Publish and check a terse fix Ticket, without exposing the private write-up.
+//! Publish and check a terse fix Ticket or Spec, without exposing the private write-up.
use std::path::PathBuf;
@@ -11,6 +11,7 @@ use crate::harness::Choice;
use crate::issue::{IssueUrl, Repo};
use crate::labels::{Edit, Label, NEEDS_TRIAGE, READY_FOR_AGENT};
use crate::prompt;
+use crate::security::reproduction::FixSize;
use crate::session::{Logs, Purpose, Sessions};
use crate::worktree::ReviewWorktree;
@@ -25,6 +26,44 @@ pub fn publish(
url: &str,
harness: &Choice,
) -> (Result