diff --git a/core/Cargo.lock b/core/Cargo.lock index 4a2afd82f6..005cdeddf3 100644 --- a/core/Cargo.lock +++ b/core/Cargo.lock @@ -3988,6 +3988,7 @@ dependencies = [ "serde_json", "serde_norway", "sha2 0.11.0", + "tempfile", "thiserror 2.0.20", "tokio-util", "url", diff --git a/core/crates/ployz-build/Cargo.toml b/core/crates/ployz-build/Cargo.toml index 252b817a5c..ee03be3210 100644 --- a/core/crates/ployz-build/Cargo.toml +++ b/core/crates/ployz-build/Cargo.toml @@ -20,5 +20,8 @@ sha2 = "0.11.0" tokio-util = "0.7.19" thiserror = "2.0.20" +[dev-dependencies] +tempfile = "3.27.0" + [lints] workspace = true diff --git a/core/crates/ployz-build/src/lib.rs b/core/crates/ployz-build/src/lib.rs index 4ce102723e..ea1db0f7d4 100644 --- a/core/crates/ployz-build/src/lib.rs +++ b/core/crates/ployz-build/src/lib.rs @@ -29,8 +29,10 @@ pub use execution::{ }; mod railpack; pub use railpack::Railpack; +mod scrub; +pub use scrub::OutputScrub; mod sealed; -pub use sealed::{sealed_file, valid_name}; +pub use sealed::{Staged, sealed_file, valid_name}; mod solve; pub use solve::PlainRenderer; @@ -172,6 +174,16 @@ enum Produce { Cache, } +/// Whether a Build solves with opened values that its cache or provenance would +/// record: a staged Sealed ARG override, or a Railpack setting planned with its value. +#[must_use] +pub fn reads_plaintext(sealed_override: Option<&Path>, railpack: &[Railpack]) -> bool { + sealed_override.is_some() + || railpack + .iter() + .any(|recipe| recipe.sealed.keys().any(|name| railpack::planned(name))) +} + /// Whether the captured environment exposes a GitHub Actions runner's cache /// service, which only `ployz build` passes through. fn github_cache(request: &Request<'_>) -> bool { @@ -329,44 +341,105 @@ impl BuildError { pub fn execute( request: &Request<'_>, cancellation: &Cancellation, + scrub: Option<&OutputScrub>, observe: &(dyn Fn(&Progress) + Sync), ) -> Result, BuildError> { execute_rendered( request, Produce::Output(request.output), cancellation, - observe, + Sinks::stderr(scrub, observe), ) } /// Upload a captured Build's cache to the GitHub Actions cache service by /// running it again, producing no image; `request.output` is not consulted. -/// Without that service in the captured environment it does nothing. Progress -/// renders to stderr only. +/// Without that service in the captured environment, or when the Build +/// [reads plaintext](reads_plaintext), it does nothing: other workflows read that +/// cache. Progress renders to stderr only. /// /// # Errors /// Returns the same failures as [`execute`]. -pub fn export_cache(request: &Request<'_>, cancellation: &Cancellation) -> Result<(), BuildError> { - if !github_cache(request) { +pub fn export_cache( + request: &Request<'_>, + cancellation: &Cancellation, + scrub: Option<&OutputScrub>, +) -> Result<(), BuildError> { + if !github_cache(request) || reads_plaintext(request.sealed_override, request.railpack) { return Ok(()); } - execute_rendered(request, Produce::Cache, cancellation, &|_| {}).map(drop) + execute_rendered( + request, + Produce::Cache, + cancellation, + Sinks::stderr(scrub, &|_| {}), + ) + .map(drop) } fn execute_rendered( request: &Request<'_>, produce: Produce, cancellation: &Cancellation, - observe: &(dyn Fn(&Progress) + Sync), + sinks: Sinks<'_>, ) -> Result, BuildError> { - let renderer = PlainRenderer::default(); - execute_produce(request, produce, Admission::wait(cancellation)?, &|event| { - if let Some(text) = renderer.render(&event) { - use std::io::Write as _; - let _ = std::io::stderr().write_all(text.as_bytes()); + let built = Admission::wait(cancellation).and_then(|admission| { + execute_produce(request, produce, admission, &|event| sinks.emit(event)) + }); + sinks.finish(); + built.map_err(|error| sinks.error(error)) +} + +/// Where a local Build's progress goes, after the scrub. +struct Sinks<'a> { + renderer: PlainRenderer, + write: &'a (dyn Fn(&[u8]) + Sync), + observe: &'a (dyn Fn(&Progress) + Sync), + scrub: Option<&'a OutputScrub>, +} + +impl<'a> Sinks<'a> { + fn stderr(scrub: Option<&'a OutputScrub>, observe: &'a (dyn Fn(&Progress) + Sync)) -> Self { + Self { + renderer: PlainRenderer::default(), + write: &|text| { + use std::io::Write as _; + let _ = std::io::stderr().write_all(text); + }, + observe, + scrub, + } + } + + fn emit(&self, event: Progress) { + match self.scrub { + Some(scrub) => scrub + .progress(event) + .iter() + .for_each(|event| self.show(event)), + None => self.show(&event), + } + } + + fn show(&self, event: &Progress) { + if let Some(text) = self.renderer.render(event) { + (self.write)(text.as_bytes()); } - observe(&event); - }) + (self.observe)(event); + } + + fn finish(&self) { + if let Some(scrub) = self.scrub { + scrub.finish().iter().for_each(|event| self.show(event)); + } + } + + fn error(&self, error: BuildError) -> BuildError { + match self.scrub { + Some(scrub) => scrub.error(error), + None => error, + } + } } /// Execute using admission acquired before source upload. The same deadline diff --git a/core/crates/ployz-build/src/railpack.rs b/core/crates/ployz-build/src/railpack.rs index 67a1526040..517662f14f 100644 --- a/core/crates/ployz-build/src/railpack.rs +++ b/core/crates/ployz-build/src/railpack.rs @@ -166,6 +166,12 @@ struct FrontendInputs { hash: String, } +/// A Railpack setting steers the plan, so it is planned with its value; any +/// other Sealed value reaches only the steps that mount it. +pub(crate) fn planned(name: &str) -> bool { + name.starts_with("RAILPACK_") +} + /// Text values ride in the script's environment and as secret files written /// here. Sealed names ride only as mounts of the files the host opened, with /// an empty placeholder so the plan still lists them. @@ -194,9 +200,19 @@ fn frontend_inputs( "Railpack sealed variables require distinct valid names".into(), )); } - command.push(' '); - command.push_str("e(&format!("{name}="))); let file = working_dir.join(crate::sealed_file(name)); + let planned = if planned(name) { + fs::read_to_string(&file) + .ok() + .filter(|value| !value.contains('\0')) + .ok_or_else(|| { + BuildError::Request(format!("sealed Railpack setting {name} was not opened")) + })? + } else { + String::new() + }; + command.push(' '); + command.push_str("e(&format!("{name}={planned}"))); secrets.push(format!("id={name},src={}", file.display())); } command.push_str(" /railpack prepare /app --plan-out /plan.json"); @@ -428,6 +444,37 @@ mod tests { fs::remove_dir_all(&directory).unwrap(); } + #[test] + fn a_sealed_railpack_setting_is_planned_with_its_opened_value() { + let capture = + std::env::temp_dir().join(format!("ployz-railpack-capture-{}", uuid::Uuid::new_v4())); + let staged = capture.join(crate::sealed_file("RAILPACK_BUILD_CMD")); + fs::create_dir_all(staged.parent().unwrap()).unwrap(); + fs::write(&staged, "npm run build -- --token marker").unwrap(); + let directory = capture.join("private/frontend"); + fs::create_dir_all(&directory).unwrap(); + let recipe = recipe( + &[], + &[("RAILPACK_BUILD_CMD", "fp-1"), ("NPM_TOKEN", "fp-2")], + ); + let planned = frontend_inputs(&recipe, &directory, &capture).unwrap(); + let prefix = planned.script.split(" /railpack ").next().unwrap(); + assert!( + prefix.contains("'RAILPACK_BUILD_CMD=npm run build -- --token marker'"), + "{prefix}" + ); + assert!(prefix.contains("'NPM_TOKEN='"), "{prefix}"); + fs::write(&staged, "npm run build").unwrap(); + assert_eq!( + planned.hash, + frontend_inputs(&recipe, &directory, &capture).unwrap().hash, + "the cache key follows the ciphertext, not the opened setting" + ); + fs::remove_file(&staged).unwrap(); + assert!(frontend_inputs(&recipe, &directory, &capture).is_err()); + fs::remove_dir_all(&capture).unwrap(); + } + #[test] fn preparation_sits_behind_the_build_bridge_firewall() { assert!(super::NETWORK.starts_with(crate::BUILD_BRIDGE_PREFIX)); diff --git a/core/crates/ployz-build/src/scrub.rs b/core/crates/ployz-build/src/scrub.rs new file mode 100644 index 0000000000..f55ee3e595 --- /dev/null +++ b/core/crates/ployz-build/src/scrub.rs @@ -0,0 +1,564 @@ +//! Hiding a Build's opened Sealed values in everything it reports: progress, its +//! rendering, the outcome and the error. + +use std::{ + io::{self, Write}, + sync::Mutex, +}; + +use ployz_core::{ + OpenedSealed, + scrub::{Channel, FrameScrub, SealedForms}, +}; + +use crate::{BuildError, Progress, remote::Outcome}; + +/// One Build's scrub. Every sink reads progress only after it. +pub struct OutputScrub { + forms: SealedForms, + frames: Mutex, +} + +/// One stream scrubber for the unattributed output and one per step, since a +/// value never continues from one step's output into another's. A step's lives +/// until the Build ends, since BuildKit can report a step complete before its +/// last log. +#[derive(Default)] +struct Frames { + output: FrameScrub, + steps: Vec<(String, FrameScrub)>, +} + +const fn channel(stderr: bool) -> Channel { + if stderr { + Channel::Stderr + } else { + Channel::Stdout + } +} + +impl OutputScrub { + #[must_use] + pub fn new(opened: &OpenedSealed) -> Self { + Self { + forms: SealedForms::compile(&opened.secrets()), + frames: Mutex::new(Frames::default()), + } + } + + /// What of `progress` can be reported now, in order. Output is held back + /// where it could begin a value, so one event may become none. + #[must_use] + pub fn progress(&self, progress: Progress) -> Vec { + let mut redacted = 0; + match progress { + Progress::Output(bytes) => { + let shown = self + .frames + .lock() + .expect("Build output scrub lock") + .output + .push(Channel::Stderr, &bytes, &self.forms); + if shown.is_empty() { + Vec::new() + } else { + vec![Progress::Output(shown)] + } + } + Progress::Step(mut step) => { + step.name = self.forms.scrub(&step.name, &mut redacted); + step.error = step + .error + .map(|error| self.forms.scrub(&error, &mut redacted)); + vec![Progress::Step(step)] + } + Progress::StepOutput { step, stderr, text } => { + let steps = &mut self.frames.lock().expect("Build output scrub lock").steps; + let index = steps + .iter() + .position(|(id, _)| id == &step) + .unwrap_or_else(|| { + steps.push((step.clone(), FrameScrub::default())); + steps.len() - 1 + }); + let shown = steps + .get_mut(index) + .expect("step was found or inserted") + .1 + .push(channel(stderr), text.as_bytes(), &self.forms); + if shown.is_empty() { + Vec::new() + } else { + vec![Progress::StepOutput { + step, + stderr, + text: String::from_utf8_lossy(&shown).into_owned(), + }] + } + } + other @ (Progress::Stage(_) | Progress::Timing { .. } | Progress::Target { .. }) => { + vec![other] + } + } + } + + /// What every stream still held when the Build ended, step by step and + /// then the unattributed output, with any trailing start of a value + /// hidden. Each ends its line, as its stream did, so the next report + /// doesn't run into it. A Build that ends early, cancelled or failed, + /// reaches this the same way as one that completes. + #[must_use] + pub fn finish(&self) -> Vec { + let frames = std::mem::take(&mut *self.frames.lock().expect("Build output scrub lock")); + let mut events: Vec = frames + .steps + .into_iter() + .flat_map(|(step, frames)| { + frames + .finish(&self.forms) + .into_iter() + .map(move |(channel, bytes)| Progress::StepOutput { + step: step.clone(), + stderr: matches!(channel, Channel::Stderr), + text: String::from_utf8_lossy(&line_ended(bytes)).into_owned(), + }) + }) + .collect(); + let tail: Vec = frames + .output + .finish(&self.forms) + .into_iter() + .flat_map(|(_, bytes)| bytes) + .collect(); + if !tail.is_empty() { + events.push(Progress::Output(line_ended(tail))); + } + events + } + + #[must_use] + pub fn outcome(&self, outcome: Outcome) -> Outcome { + let mut redacted = 0; + match outcome { + Outcome::Failed { + work, + stage, + message, + } => Outcome::Failed { + work, + stage, + message: self.forms.scrub(&message, &mut redacted), + }, + Outcome::Unknown { + work, + stage, + message, + } => Outcome::Unknown { + work, + stage, + message: self.forms.scrub(&message, &mut redacted), + }, + other @ (Outcome::Images { .. } + | Outcome::Validated { .. } + | Outcome::Published { .. } + | Outcome::CapabilitiesChecked { .. }) => other, + } + } + + #[must_use] + pub fn error(&self, error: BuildError) -> BuildError { + let scrub = |text: String| self.forms.scrub(&text, &mut 0); + match error { + BuildError::AtStage { stage, failure } => BuildError::AtStage { + stage, + failure: Box::new(self.error(*failure)), + }, + BuildError::Prerequisite(text) => BuildError::Prerequisite(scrub(text)), + BuildError::Request(text) => BuildError::Request(scrub(text)), + BuildError::Docker { action, diagnostic } => BuildError::Docker { + action, + diagnostic: scrub(diagnostic), + }, + BuildError::Step { step, error } => BuildError::Step { + step: scrub(step), + error: scrub(error), + }, + BuildError::UncertainTermination(text) => BuildError::UncertainTermination(scrub(text)), + BuildError::Result(text) => BuildError::Result(scrub(text)), + other @ (BuildError::Busy | BuildError::Cancelled | BuildError::TimedOut(_)) => other, + } + } + + /// Ask a GitHub Actions runner to mask each form in its own log too, behind the + /// scrub. A form spanning lines can't be one mask; its rows are masked instead. + /// + /// # Errors + /// Fails when `out` does. + pub fn mask_github(&self, out: &mut dyn Write) -> io::Result<()> { + for form in self.forms.long_forms() { + if let Ok(form) = std::str::from_utf8(form) + && !form.contains(['\n', '\r']) + { + writeln!(out, "::add-mask::{form}")?; + } + } + Ok(()) + } +} + +fn line_ended(mut bytes: Vec) -> Vec { + if bytes.last().is_some_and(|byte| *byte != b'\n') { + bytes.push(b'\n'); + } + bytes +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + use crate::BuildStep; + + const STEP: &str = "sha256:step-one"; + + fn scrub(value: &str) -> OutputScrub { + OutputScrub::new(&OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".to_owned(), value.to_owned())]), + Vec::new(), + )) + } + + fn step_output(text: &str) -> Progress { + Progress::StepOutput { + step: STEP.into(), + stderr: false, + text: text.into(), + } + } + + fn completed() -> Progress { + Progress::Step(BuildStep { + id: STEP.into(), + name: "RUN npm install".into(), + started: Some("t0".into()), + completed: Some("t1".into()), + cached: false, + error: None, + }) + } + + /// Every event's text in order, with the shown step output joined. + fn report(scrub: &OutputScrub, events: Vec) -> (String, Vec) { + let mut shown = Vec::new(); + for event in events { + shown.extend(scrub.progress(event)); + } + shown.extend(scrub.finish()); + let text = shown + .iter() + .filter_map(|event| { + if let Progress::StepOutput { text, .. } = event { + Some(text.as_str()) + } else { + None + } + }) + .collect(); + (text, shown) + } + + #[test] + fn a_value_cut_between_two_step_output_events_never_shows() { + let value = "opened-token-0123456789"; + let scrub = scrub(value); + let padding = "x".repeat(64 * 1024 - 7); + let first = format!("{padding}{}", &value[..7]); + let second = format!("{} and more\n", &value[7..]); + let (text, shown) = report( + &scrub, + vec![step_output(&first), step_output(&second), completed()], + ); + assert!(!text.contains(value), "{text}"); + assert_eq!(text, format!("{padding}[secret NPM_TOKEN] and more\n")); + assert!(matches!(shown.last(), Some(Progress::Step(step)) if step.completed.is_some())); + } + + #[test] + fn a_multiline_value_cut_at_its_own_newline_never_shows() { + let value = "line1-secret\nline2-secret"; + let scrub = scrub(value); + let (text, _) = report( + &scrub, + vec![ + step_output("cert: line1-secret\n"), + step_output("line2-secret end\n"), + completed(), + ], + ); + assert!(!text.contains("line1-secret"), "{text}"); + assert!(!text.contains("line2-secret"), "{text}"); + assert_eq!(text, "cert: [secret NPM_TOKEN] end\n"); + } + + #[test] + fn plain_output_arrives_whole_and_in_order_and_a_held_tail_leaves_at_the_end() { + let scrub = scrub("opened-token-0123456789"); + let events = vec![ + step_output("one\n"), + step_output("two "), + step_output("three\n"), + Progress::StepOutput { + step: "sha256:step-two".into(), + stderr: true, + text: "warning: slow".into(), + }, + step_output("four"), + ]; + let (text, shown) = report(&scrub, events); + assert_eq!(text, "one\ntwo three\nfour\nwarning: slow\n"); + let order: Vec<(&str, bool)> = shown + .iter() + .filter_map(|event| { + if let Progress::StepOutput { step, stderr, .. } = event { + Some((step.as_str(), *stderr)) + } else { + None + } + }) + .collect(); + // Held text leaves as late as it must: "two " waits for its line end, and + // what each step still holds leaves at the end, step by step. + assert_eq!( + order, + [ + (STEP, false), + (STEP, false), + (STEP, false), + ("sha256:step-two", true) + ] + ); + } + + #[test] + fn held_step_tails_follow_first_output_order_instead_of_digest_order() { + let output = |step: &str, text: &str| Progress::StepOutput { + step: step.into(), + stderr: false, + text: text.into(), + }; + let (text, shown) = report( + &scrub("opened-token-0123456789"), + vec![ + output("sha256:z-first", "first "), + output("sha256:a-second", "second"), + output("sha256:z-first", "tail"), + ], + ); + assert_eq!(text, "first tail\nsecond\n"); + let steps: Vec<_> = shown + .iter() + .filter_map(|event| { + if let Progress::StepOutput { step, .. } = event { + Some(step.as_str()) + } else { + None + } + }) + .collect(); + assert_eq!(steps, ["sha256:z-first", "sha256:a-second"]); + } + + #[test] + fn a_scrubbed_tail_ends_its_line_where_its_stream_ended() { + let value = "opened-token-0123456789"; + let scrub = scrub(value); + let mut shown = scrub.progress(step_output(value)); + shown.extend(scrub.progress(Progress::Output(value.as_bytes().to_vec()))); + shown.extend(scrub.finish()); + let texts: Vec = shown + .into_iter() + .filter_map(|event| match event { + Progress::StepOutput { text, .. } => Some(text), + Progress::Output(bytes) => Some(String::from_utf8(bytes).unwrap()), + Progress::Step(_) + | Progress::Stage(_) + | Progress::Timing { .. } + | Progress::Target { .. } => None, + }) + .collect(); + assert_eq!(texts, ["[secret NPM_TOKEN]\n", "[secret NPM_TOKEN]\n"]); + } + + /// A step's last log can arrive after BuildKit reports it complete, so a + /// value cut by the completion is still one value. + #[test] + fn a_value_cut_by_its_steps_completion_never_shows() { + for (value, before, after, expected) in [ + ( + "opened-token-0123456789", + "token is opened-", + "token-0123456789 and more\n", + "token is [secret NPM_TOKEN] and more\n", + ), + ( + "s3cr", + "NPM_TOKEN=", + "s3cr\n", + "NPM_TOKEN=[secret NPM_TOKEN]\n", + ), + ] { + let scrub = scrub(value); + let (text, shown) = report( + &scrub, + vec![step_output(before), completed(), step_output(after)], + ); + assert!(!text.contains(value), "{text}"); + assert_eq!(text, expected); + assert!( + shown + .iter() + .any(|event| matches!(event, Progress::Step(step) if step.completed.is_some())) + ); + } + } + + #[test] + fn a_character_split_by_parser_completion_still_scrubs_the_whole_value() { + use crate::{Admission, HostPolicy, Output, Request, Target, execute_admitted}; + use base64::Engine as _; + use std::{fs, os::unix::fs::PermissionsExt as _}; + + let value = "🐓opened-token-0123456789"; + let (head, tail) = value.as_bytes().split_at(2); + let record = |bytes: &[u8]| { + format!( + "{{\"logs\":[{{\"vertex\":\"{STEP}\",\"stream\":1,\"data\":\"{}\"}}]}}\n", + base64::engine::general_purpose::STANDARD.encode(bytes) + ) + }; + for sealed in [false, true] { + let root = tempfile::tempdir().unwrap(); + fs::set_permissions(root.path(), fs::Permissions::from_mode(0o700)).unwrap(); + let docker = root.path().join("docker"); + fs::write( + &docker, + format!( + r#"#!/bin/sh +case "$1 $2" in + 'context show') echo default ;; + 'info --format') echo '{{"OSType":"linux","CgroupVersion":"2","Architecture":"amd64","DriverStatus":[["driver-type","io.containerd.snapshotter.v1"]]}}' ;; + 'buildx ls') echo '{{"Name":"{}","Nodes":[{{"Status":"running","Platforms":["linux/amd64"]}}]}}' ;; + 'buildx bake') cat solve-events ;; +esac +exit 0 +"#, + crate::builder_name() + ), + ) + .unwrap(); + fs::set_permissions(&docker, fs::Permissions::from_mode(0o700)).unwrap(); + fs::write( + root.path().join("solve-events"), + format!( + "{}{{\"vertexes\":[{{\"digest\":\"{STEP}\",\"completed\":\"t1\"}}]}}\n{}", + record(head), + record(&[tail, b"\n"].concat()) + ), + ) + .unwrap(); + let admission = Admission::try_acquire_with(&HostPolicy { + state_directory: root.path().to_owned(), + configuration_file: root.path().join("build.yaml"), + docker: docker.clone(), + ..Default::default() + }) + .unwrap(); + let request = Request { + image_contexts: &BTreeMap::new(), + compose_file: &root.path().join("compose.yaml"), + sealed_override: None, + working_dir: root.path(), + environment: &BTreeMap::new(), + docker: Some(&docker), + targets: &[Target { + name: "app".into(), + platforms: Vec::new(), + }], + railpack: &[], + build_args: &[], + output: Output::Validate, + no_cache: false, + pull: false, + sealed, + }; + let events = Mutex::new(Vec::new()); + execute_admitted(&request, admission, &|event| { + events.lock().unwrap().push(event); + }) + .unwrap(); + let events = events.into_inner().unwrap(); + if !sealed { + let text: Vec<&str> = events + .iter() + .filter_map(|event| { + if let Progress::StepOutput { text, .. } = event { + Some(text.as_str()) + } else { + None + } + }) + .collect(); + assert_eq!( + text, + ["\u{FFFD}", "\u{FFFD}\u{FFFD}opened-token-0123456789\n"] + ); + continue; + } + let (text, _) = report(&scrub(value), events); + assert!(!text.contains(value), "{text}"); + assert!(!text.contains("opened-token-0123456789"), "{text}"); + assert_eq!(text, "[secret NPM_TOKEN]\n"); + } + } + + #[test] + fn plain_text_held_across_a_steps_completion_shows_whole() { + // The value starts with the byte `Done` ends with, so a stream ended at + // the completion would have hidden it as the start of the value. + let scrub = scrub("e-opened-token-0123456789"); + let (text, _) = report( + &scrub, + vec![step_output("Done"), completed(), step_output(" in 1.2s\n")], + ); + assert_eq!(text, "Done in 1.2s\n"); + } + + #[test] + fn a_value_begun_when_the_build_stops_stays_hidden() { + let value = "opened-token-0123456789"; + let scrub = scrub(value); + let (text, _) = report(&scrub, vec![step_output("token is opened-tok")]); + assert!(!text.contains("opened-tok"), "{text}"); + assert_eq!(text, "token is [secret NPM_TOKEN]\n"); + } + + #[test] + fn a_runner_masks_each_single_line_form_and_no_short_value() { + let scrub = OutputScrub::new(&OpenedSealed::new( + BTreeMap::from([ + ("CERT".to_owned(), "line1-secret\nline2-secret".to_owned()), + ("PIN".to_owned(), "s3cr".to_owned()), + ]), + Vec::new(), + )); + let mut out = Vec::new(); + scrub.mask_github(&mut out).unwrap(); + let out = String::from_utf8(out).unwrap(); + let masks: Vec<&str> = out.lines().collect(); + assert!(masks.contains(&"::add-mask::line1-secret"), "{out}"); + assert!(masks.contains(&"::add-mask::line2-secret"), "{out}"); + assert!(masks.iter().all(|mask| mask.starts_with("::add-mask::"))); + assert!(!out.contains("s3cr"), "{out}"); + } +} diff --git a/core/crates/ployz-build/src/sealed.rs b/core/crates/ployz-build/src/sealed.rs index 91dc68f4f1..b6fd63302b 100644 --- a/core/crates/ployz-build/src/sealed.rs +++ b/core/crates/ployz-build/src/sealed.rs @@ -1,7 +1,8 @@ //! Opened Sealed values live as 0600 files for one solve and reach BuildKit //! as secret mounts and declared Dockerfile ARGs. The Builder writes the files. -use crate::BuildError; +use crate::{BuildError, Target}; +use ployz_core::OpenedSealed; use std::{ collections::BTreeMap, fs, io, @@ -28,17 +29,39 @@ pub fn valid_name(name: &str) -> bool { /// Opened values written for one solve. Dropping removes them on every /// outcome, including a retained Unknown root and a dropped cancelled Build. -pub(crate) struct OpenedFiles { +pub struct Staged { directory: Option, files: Vec, + compose_override: Option, } -impl OpenedFiles { - pub(crate) fn args( +impl Staged { + /// Write `opened` under the capture at `root`: a file per value, and the Compose + /// override that passes the declared Dockerfile ARGs of `targets`. + /// + /// # Errors + /// Fails on a name that can't be a secret id, an ARG nothing opened, or a write. + pub fn stage( + root: &Path, + opened: &OpenedSealed, + targets: &[Target], + ) -> Result { + let mut staged = Self::write(root, opened)?; + staged.compose_override = staged.args(root, targets, opened)?; + Ok(staged) + } + + /// The Compose file that passes opened values as ARGs. + #[must_use] + pub fn compose_override(&self) -> Option<&Path> { + self.compose_override.as_deref() + } + + fn args( &mut self, root: &Path, - definition: &crate::remote::Definition, - opened: &BTreeMap, + targets: &[Target], + opened: &OpenedSealed, ) -> Result, BuildError> { let bytes = match fs::read(root.join("private/sealed-args.json")) { Ok(bytes) => bytes, @@ -49,11 +72,7 @@ impl OpenedFiles { .map_err(|_| BuildError::Request("invalid Sealed Build argument names".into()))?; let mut services = BTreeMap::new(); for (target, names) in declared { - if !definition - .targets - .iter() - .any(|admitted| admitted.name == target) - { + if !targets.iter().any(|admitted| admitted.name == target) { return Err(BuildError::Request( "Sealed arguments name an unadmitted target".into(), )); @@ -87,13 +106,11 @@ impl OpenedFiles { Ok(Some(path)) } - pub(crate) fn write( - root: &Path, - opened: &BTreeMap, - ) -> Result { + fn write(root: &Path, opened: &OpenedSealed) -> Result { let mut files = Self { directory: None, files: Vec::new(), + compose_override: None, }; if opened.is_empty() { return Ok(files); @@ -104,7 +121,7 @@ impl OpenedFiles { .create(&directory) .map_err(io_error)?; files.directory = Some(directory); - for (name, value) in opened { + for (name, value) in opened.values() { if !valid_name(name) { return Err(BuildError::Request(format!( "{name:?} cannot be a Build secret name" @@ -124,7 +141,7 @@ impl OpenedFiles { } } -impl Drop for OpenedFiles { +impl Drop for Staged { fn drop(&mut self) { for file in &self.files { let _ = fs::remove_file(file); @@ -143,6 +160,10 @@ fn io_error(error: io::Error) -> BuildError { mod tests { use super::*; + fn opened(values: BTreeMap) -> OpenedSealed { + OpenedSealed::new(values, Vec::new()) + } + struct Root(PathBuf); impl Root { fn new() -> Self { @@ -164,11 +185,11 @@ mod tests { fn opened_files_are_private_and_vanish_when_the_guard_drops() { use std::os::unix::fs::PermissionsExt as _; let root = Root::new(); - let opened = BTreeMap::from([ + let values = BTreeMap::from([ ("NPM_TOKEN".to_owned(), "opened-token".to_owned()), ("OTHER".to_owned(), "two".to_owned()), ]); - let guard = OpenedFiles::write(root.path(), &opened).unwrap(); + let guard = Staged::write(root.path(), &opened(values)).unwrap(); let token = root.path().join(sealed_file("NPM_TOKEN")); assert_eq!(fs::read_to_string(&token).unwrap(), "opened-token"); assert_eq!( @@ -191,25 +212,18 @@ mod tests { #[test] fn no_opened_values_leave_private_material_untouched() { let root = Root::new(); - let _guard = OpenedFiles::write(root.path(), &BTreeMap::new()).unwrap(); + let _guard = Staged::write(root.path(), &opened(BTreeMap::new())).unwrap(); assert_eq!( fs::read_dir(root.path().join("private")).unwrap().count(), 0 ); } - fn definition() -> crate::remote::Definition { - crate::remote::Definition { - targets: vec![crate::Target { - name: "api".into(), - platforms: Vec::new(), - }], - retained_tags: Vec::new(), - image_contexts: BTreeMap::new(), - output: crate::Output::Load, - no_cache: false, - pull: false, - } + fn targets() -> Vec { + vec![Target { + name: "api".into(), + platforms: Vec::new(), + }] } #[test] @@ -223,15 +237,12 @@ mod tests { ) .unwrap(); let value = "literal $TOKEN ${HOME} $$ \\\"\nsecond line"; - let opened = BTreeMap::from([ + let opened = opened(BTreeMap::from([ ("NPM_TOKEN".into(), value.into()), ("MOUNT_ONLY".into(), "mount-only-value".into()), - ]); - let mut guard = OpenedFiles::write(root.path(), &opened).unwrap(); - let file = guard - .args(root.path(), &definition(), &opened) - .unwrap() - .unwrap(); + ])); + let guard = Staged::stage(root.path(), &opened, &targets()).unwrap(); + let file = guard.compose_override().unwrap().to_owned(); let encoded = fs::read_to_string(&file).unwrap(); let compose: serde_json::Value = serde_json::from_str(&encoded.replace("$$", "$")).unwrap(); assert_eq!( @@ -255,16 +266,14 @@ mod tests { #[test] fn invalid_arg_material_fails_without_leaving_opened_files() { let root = Root::new(); - let opened = BTreeMap::from([("NPM_TOKEN".into(), "dummy".into())]); + let opened = opened(BTreeMap::from([("NPM_TOKEN".into(), "dummy".into())])); for contents in [ "bad json", r#"{"other":["NPM_TOKEN"]}"#, r#"{"api":["MISSING"]}"#, ] { fs::write(root.path().join("private/sealed-args.json"), contents).unwrap(); - let mut guard = OpenedFiles::write(root.path(), &opened).unwrap(); - assert!(guard.args(root.path(), &definition(), &opened).is_err()); - drop(guard); + assert!(Staged::stage(root.path(), &opened, &targets()).is_err()); assert!(!root.path().join(DIRECTORY).exists()); } } @@ -273,11 +282,8 @@ mod tests { fn names_that_cannot_be_files_or_secret_ids_are_refused() { let root = Root::new(); for name in ["", "../escape", "a/b", "with space", "a=b"] { - let opened = BTreeMap::from([(name.to_owned(), "x".to_owned())]); - assert!( - OpenedFiles::write(root.path(), &opened).is_err(), - "{name:?}" - ); + let opened = opened(BTreeMap::from([(name.to_owned(), "x".to_owned())])); + assert!(Staged::write(root.path(), &opened).is_err(), "{name:?}"); } assert!(!root.path().join(DIRECTORY).exists()); } diff --git a/core/crates/ployz-build/src/tests.rs b/core/crates/ployz-build/src/tests.rs index 39cb2d06fd..c6b58daec8 100644 --- a/core/crates/ployz-build/src/tests.rs +++ b/core/crates/ployz-build/src/tests.rs @@ -567,7 +567,7 @@ esac pull: false, sealed: false, }; - execute(&request, &Cancellation::new(), &|_| ()).unwrap(); + execute(&request, &Cancellation::new(), None, &|_| ()).unwrap(); std::fs::remove_dir_all(directory).unwrap(); Command::new("kill") .args([&signal, &std::process::id().to_string()]) @@ -811,18 +811,43 @@ fn repository_reference_rejects_a_malformed_service_image() { } #[test] -fn a_cache_export_without_a_runner_cache_runs_nothing() { - let environment = BTreeMap::new(); +fn a_cache_export_without_a_runner_cache_or_after_reading_plaintext_runs_nothing() { + let runner = BTreeMap::from([("ACTIONS_RUNTIME_TOKEN".to_owned(), "token".to_owned())]); + let planned = [Railpack { + name: "api".into(), + context: "source/api".into(), + variables: BTreeMap::new(), + sealed: BTreeMap::from([("RAILPACK_BUILD_CMD".to_owned(), "f".repeat(64))]), + refresh_cache: false, + }]; + for (environment, sealed_override, railpack) in [ + (BTreeMap::new(), None, &[][..]), + ( + runner.clone(), + Some(Path::new("private/sealed/args.compose.yaml")), + &[][..], + ), + (runner, None, &planned[..]), + ] { + a_cache_export_runs_nothing(&environment, sealed_override, railpack); + } +} + +fn a_cache_export_runs_nothing( + environment: &BTreeMap, + sealed_override: Option<&Path>, + railpack: &[Railpack], +) { let targets = [target("api", None)]; let request = Request { image_contexts: &BTreeMap::new(), compose_file: Path::new("compose.yaml"), - sealed_override: None, + sealed_override, working_dir: Path::new("/nonexistent"), - environment: &environment, + environment, docker: Some(Path::new("/nonexistent/docker")), targets: &targets, - railpack: &[], + railpack, build_args: &[], output: Output::Load, no_cache: false, @@ -830,7 +855,7 @@ fn a_cache_export_without_a_runner_cache_runs_nothing() { sealed: false, }; // Any Docker call would fail: this Docker does not exist. - assert_eq!(export_cache(&request, &Cancellation::new()), Ok(())); + assert_eq!(export_cache(&request, &Cancellation::new(), None), Ok(())); } #[test] @@ -919,9 +944,12 @@ fn railpack_delivers_a_sealed_value_to_a_build_step_and_keeps_it_out_of_the_imag format!("services:\n web:\n image: {image}\n build:\n context: source/app\n"), ) .unwrap(); - let opened = BTreeMap::from([("NPM_TOKEN".to_owned(), value.to_owned())]); - let _files = crate::sealed::OpenedFiles::write(&directory, &opened).unwrap(); + let opened = ployz_core::OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".to_owned(), value.to_owned())]), + Vec::new(), + ); let targets = [target("web", None)]; + let _files = Staged::stage(&directory, &opened, &targets).unwrap(); let railpack = [Railpack { name: "web".into(), context: "source/app".into(), @@ -1194,7 +1222,10 @@ fn dockerfile_sealed_args_reach_the_image_and_invalidate_the_cache() { .execute( &definition, None, - &BTreeMap::from([("NPM_TOKEN".into(), value.into())]), + Some(&ployz_core::OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".into(), value.into())]), + Vec::new(), + )), &|progress| match progress { Progress::Step(step) => steps.lock().unwrap().push(step), Progress::Target { .. } => no_plaintext(&upload_root, value), @@ -1266,3 +1297,78 @@ fn dockerfile_sealed_args_reach_the_image_and_invalidate_the_cache() { ]); std::fs::remove_dir_all(directory).unwrap(); } + +#[test] +fn the_rendered_stderr_and_the_observer_both_read_only_scrubbed_progress() { + let value = "opened-token-0123456789"; + let scrub = OutputScrub::new(&ployz_core::OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".to_owned(), value.to_owned())]), + Vec::new(), + )); + let written = std::sync::Mutex::new(Vec::new()); + let observed = std::sync::Mutex::new(Vec::new()); + let sinks = Sinks { + renderer: PlainRenderer::default(), + write: &|text| written.lock().unwrap().extend_from_slice(text), + observe: &|event| observed.lock().unwrap().push(format!("{event:?}")), + scrub: Some(&scrub), + }; + let output = |text: &str| Progress::StepOutput { + step: "sha256:one".into(), + stderr: false, + text: text.into(), + }; + sinks.emit(Progress::Step(BuildStep { + id: "sha256:one".into(), + name: format!("RUN echo {value}"), + started: Some("t0".into()), + completed: None, + cached: false, + error: None, + })); + sinks.emit(output("token opened-tok")); + sinks.emit(output("en-0123456789 ok\n")); + sinks.emit(output("ends opened-")); + sinks.finish(); + let error = sinks.error( + BuildError::Step { + step: "RUN".into(), + error: format!("exit 1: {value}"), + } + .at(Stage::Building), + ); + + let written = String::from_utf8(written.into_inner().unwrap()).unwrap(); + let observed = observed.into_inner().unwrap().concat(); + for sink in [&written, &observed, &error.to_string()] { + assert!(!sink.contains("opened-tok"), "{sink}"); + assert!(sink.contains("[secret NPM_TOKEN]"), "{sink}"); + } +} + +#[test] +fn without_a_scrub_both_sinks_read_progress_as_it_came() { + let written = std::sync::Mutex::new(Vec::new()); + let observed = std::sync::Mutex::new(Vec::new()); + let sinks = Sinks { + renderer: PlainRenderer::default(), + write: &|text| written.lock().unwrap().extend_from_slice(text), + observe: &|event| observed.lock().unwrap().push(event.clone()), + scrub: None, + }; + sinks.emit(Progress::Output( + b"plain opened-token-0123456789\n".to_vec(), + )); + sinks.finish(); + assert_eq!( + observed.into_inner().unwrap(), + [Progress::Output( + b"plain opened-token-0123456789\n".to_vec() + )] + ); + assert!( + String::from_utf8(written.into_inner().unwrap()) + .unwrap() + .contains("plain opened-token-0123456789") + ); +} diff --git a/core/crates/ployz-build/src/upload.rs b/core/crates/ployz-build/src/upload.rs index 23f5073dfd..c3fd319f91 100644 --- a/core/crates/ployz-build/src/upload.rs +++ b/core/crates/ployz-build/src/upload.rs @@ -5,6 +5,7 @@ use crate::{ Admission, BuildError, Progress, Request, remote::{CHUNK_SIZE, Definition, Input, Kind}, }; +use ployz_core::OpenedSealed; use std::{ collections::{BTreeMap, BTreeSet, VecDeque}, fs::{self, File}, @@ -134,7 +135,7 @@ impl AdmittedUpload { self, definition: &Definition, docker: Option<&Path>, - opened: &BTreeMap, + opened: Option<&OpenedSealed>, progress: &(dyn Fn(Progress) + Sync), ) -> Result { let _ownership = self.admission.lock.clone(); @@ -336,25 +337,27 @@ impl CompletedUpload { definition: &Definition, admission: Admission, docker: Option<&Path>, - opened: &BTreeMap, + opened: Option<&OpenedSealed>, progress: &(dyn Fn(Progress) + Sync), ) -> Result { let mut upload = self.0; + let none = OpenedSealed::new(BTreeMap::new(), Vec::new()); + let opened = opened.unwrap_or(&none); // Declared after `upload` so the files go before the root does. - let mut opened_files = crate::sealed::OpenedFiles::write(&upload.root, opened)?; + let staged = crate::Staged::stage(&upload.root, opened, &definition.targets)?; let railpack = crate::received_recipe::validate_capture( &upload.root, definition, crate::received_recipe::SealedFiles::Opened, ) .map_err(|error| BuildError::Request(error.to_string()))?; - let sealed_override = opened_files.args(&upload.root, definition, opened)?; + let sealed_override = staged.compose_override(); for (path, mode) in upload.directories.iter().rev() { fs::set_permissions(upload.root.join(path), fs::Permissions::from_mode(*mode)) .map_err(|error| BuildError::Request(io_error(error).to_string()))?; } let mut environment = environment(&upload.root); - if sealed_override.is_some() { + if crate::reads_plaintext(sealed_override, &railpack) { // Buildx provenance metadata records build argument values. environment.insert("BUILDX_METADATA_PROVENANCE".into(), "disabled".into()); } @@ -368,7 +371,7 @@ impl CompletedUpload { image_contexts: &definition.image_contexts, railpack: &railpack, compose_file: Path::new("compose.yaml"), - sealed_override: sealed_override.as_deref(), + sealed_override, working_dir: &upload.root, environment: &environment, docker, diff --git a/core/crates/ployz-build/tests/host_policy.rs b/core/crates/ployz-build/tests/host_policy.rs index d21f35f4af..75894668e7 100644 --- a/core/crates/ployz-build/tests/host_policy.rs +++ b/core/crates/ployz-build/tests/host_policy.rs @@ -465,11 +465,14 @@ fn opened_values_are_private_files_for_the_solve_and_vanish_when_it_fails() { let admission = Admission::try_acquire_with(&host.policy).unwrap(); let mut upload = admission.upload().unwrap(); remote::upload(&capture, |input| upload.accept(input)).unwrap(); - let opened = BTreeMap::from([("NPM_TOKEN".to_owned(), "opened-token".to_owned())]); + let opened = ployz_core::OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".to_owned(), "opened-token".to_owned())]), + Vec::new(), + ); let Err(error) = upload.execute( &definition(Output::Load), Some(&host.policy.docker), - &opened, + Some(&opened), &|_| {}, ) else { panic!("a failed solve succeeded"); @@ -501,12 +504,15 @@ fn opened_values_vanish_from_a_retained_unknown_root() { let admission = Admission::try_acquire_with(&host.policy).unwrap(); let mut upload = admission.upload().unwrap(); remote::upload(&capture, |input| upload.accept(input)).unwrap(); - let opened = BTreeMap::from([("NPM_TOKEN".to_owned(), "opened-token".to_owned())]); + let opened = ployz_core::OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".to_owned(), "opened-token".to_owned())]), + Vec::new(), + ); let error = upload .execute( &definition(Output::Load), Some(&host.policy.docker), - &opened, + Some(&opened), &|_| {}, ) .err() @@ -537,7 +543,7 @@ fn a_capture_naming_a_sealed_value_nobody_opened_is_refused_before_docker() { let Err(error) = upload.execute( &definition(Output::Load), Some(&host.policy.docker), - &BTreeMap::new(), + None, &|_| {}, ) else { panic!("docker ran without the opened file"); diff --git a/core/crates/ployz-core/src/domain/build_grant.rs b/core/crates/ployz-core/src/domain/build_grant.rs index 02e8416c83..df81e4a338 100644 --- a/core/crates/ployz-core/src/domain/build_grant.rs +++ b/core/crates/ployz-core/src/domain/build_grant.rs @@ -113,4 +113,17 @@ mod tests { let error = BuildGrant::parse(format!("{text} ")).unwrap_err(); assert!(!format!("{error}").contains(&text[PREFIX.len()..])); } + + /// The fixture the `getployz/build` Action assembles its grant against. + #[test] + fn a_holder_assembles_the_contract_grant_string() { + let hex = "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60"; + let seed: [u8; 32] = std::array::from_fn(|index| { + u8::from_str_radix(&hex[index * 2..index * 2 + 2], 16).unwrap() + }); + assert_eq!( + BuildGrant::new(ManagementIdentity::from_bytes([1; 32]), seed).to_secret_string(), + "ployzgrant1:AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQGdYbGd7_1aYLqESvSS7CzEREnFaXsyaRlwO6wDHK5_YA" + ); + } } diff --git a/core/crates/ployz-core/src/lib.rs b/core/crates/ployz-core/src/lib.rs index 56841fabef..29596775be 100644 --- a/core/crates/ployz-core/src/lib.rs +++ b/core/crates/ployz-core/src/lib.rs @@ -27,6 +27,7 @@ pub mod rpc; mod rpc_catalog; pub mod scrub; mod sealed; +mod sealed_record; mod sealing; pub mod service; mod storage_capacity; @@ -52,6 +53,7 @@ pub use release_mirror::*; pub use routing::*; pub use rpc::*; pub use sealed::*; +pub use sealed_record::*; pub use sealing::*; pub use service::*; pub use storage_capacity::*; diff --git a/core/crates/ployz-core/src/rpc/build_grant.rs b/core/crates/ployz-core/src/rpc/build_grant.rs index 8f7e0c08be..94d6f95d75 100644 --- a/core/crates/ployz-core/src/rpc/build_grant.rs +++ b/core/crates/ployz-core/src/rpc/build_grant.rs @@ -8,6 +8,17 @@ use ts_rs::TS; pub struct MintBuildGrantRequest { /// The only repository the push may write, as Docker names it (`ployz-build/web`). pub repository: crate::BuildGrantRepository, + /// The pusher's own public key, which becomes the grant's id. Without one the + /// Machine makes the key and returns its secret. Only a daemon advertising + /// [`crate::BUILD_GRANT_HOLDER_CAPABILITY`] reads it. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub holder: Option, + /// The built Service's Sealed values, still sealed. The Machine opens them at mint + /// and serves them only to `holder`, which must be set. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub sealed: Option, } /// A minted grant. The Machine keeps it in memory only; a restart ends it. @@ -15,13 +26,48 @@ pub struct MintBuildGrantRequest { pub struct BuildGrantMinted { /// Handle for [`InspectBuildGrantRequest`]; not secret. pub id: crate::BuildGrantId, - /// Secret-bearing grant for the pusher only. - #[ts(type = "string")] - pub grant: crate::BuildGrant, + #[serde(flatten)] + pub key: MintedKey, /// The grant ends by itself this long after minting. pub expires_in_seconds: u64, } +/// What the pusher dials a minted grant with. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] +#[serde(untagged)] +pub enum MintedKey { + /// The Machine made the key: the grant's secret, for the pusher only. + Grant { + #[ts(type = "string")] + grant: crate::BuildGrant, + }, + /// The holder made the key and keeps it: only the Machine to dial. + Holder { + #[ts(type = "string")] + #[serde(with = "machine_hex")] + machine: crate::ManagementIdentity, + }, +} + +mod machine_hex { + use serde::{Deserialize, Deserializer, Serializer, de::Error as _}; + + use crate::ManagementIdentity; + + pub(super) fn serialize( + machine: &ManagementIdentity, + serializer: S, + ) -> Result { + serializer.serialize_str(&machine.to_hex()) + } + + pub(super) fn deserialize<'de, D: Deserializer<'de>>( + deserializer: D, + ) -> Result { + ManagementIdentity::parse_hex(&String::deserialize(deserializer)?).map_err(D::Error::custom) + } +} + /// Read what a Build Grant received. Reading neither spends the grant nor shows its /// secret; the grant ends by expiring. An unknown id, expired or lost to a daemon /// restart, is `not_found`. @@ -41,3 +87,59 @@ pub struct BuildGrantInspected { /// The grant ends by itself this long from now. pub expires_in_seconds: u64, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_minted_grant_reads_as_a_secret_or_a_machine_to_dial() { + let machine = crate::ManagementIdentity::from_bytes([1; 32]); + let grant = crate::BuildGrant::new(machine, [2; 32]); + let id = crate::BuildGrantId::parse("d".repeat(64)).unwrap(); + let legacy = serde_json::json!({ + "id": id, "grant": grant.to_secret_string(), "expires_in_seconds": 10, + }); + assert_eq!( + serde_json::from_value::(legacy.clone()).unwrap(), + BuildGrantMinted { + id, + key: MintedKey::Grant { grant }, + expires_in_seconds: 10, + } + ); + let held = BuildGrantMinted { + id, + key: MintedKey::Holder { machine }, + expires_in_seconds: 10, + }; + let wire = serde_json::to_value(&held).unwrap(); + assert_eq!( + wire, + serde_json::json!({"id": id, "machine": "01".repeat(32), "expires_in_seconds": 10}) + ); + assert_eq!( + serde_json::from_value::(wire).unwrap(), + held + ); + } + + #[test] + fn a_mint_request_without_a_holder_reads_as_before() { + let repository = crate::BuildGrantRepository::parse("ployz-build/web").unwrap(); + let request: MintBuildGrantRequest = + serde_json::from_value(serde_json::json!({"repository": "ployz-build/web"})).unwrap(); + assert_eq!( + request, + MintBuildGrantRequest { + repository, + holder: None, + sealed: None, + } + ); + assert_eq!( + serde_json::to_value(&request).unwrap(), + serde_json::json!({"repository": "ployz-build/web"}) + ); + } +} diff --git a/core/crates/ployz-core/src/rpc_catalog.rs b/core/crates/ployz-core/src/rpc_catalog.rs index 322baaaf3f..f2f871d1c8 100644 --- a/core/crates/ployz-core/src/rpc_catalog.rs +++ b/core/crates/ployz-core/src/rpc_catalog.rs @@ -117,6 +117,8 @@ macro_rules! rpc_catalog { ENV_SEALED_CAPABILITY: ("ployz.env.sealed.v1", Always), /// The daemon opens a Build's Sealed values as secret files and keeps them out of its output. BUILD_SEALED_CAPABILITY: ("ployz.build.sealed.v1", Always), + /// The daemon mints a Build Grant for a holder's own key and returns no grant secret. + BUILD_GRANT_HOLDER_CAPABILITY: ("ployz.build.grant.holder.v1", Always), } } }; diff --git a/core/crates/ployz-core/src/scrub.rs b/core/crates/ployz-core/src/scrub.rs index 035635d53a..ab9fbaca79 100644 --- a/core/crates/ployz-core/src/scrub.rs +++ b/core/crates/ployz-core/src/scrub.rs @@ -120,6 +120,11 @@ impl SealedForms { Self::compile(&SealedValues::none()) } + /// Each form matched on its own, without a label. + pub fn long_forms(&self) -> impl Iterator { + self.names.keys().map(Vec::as_slice) + } + /// Replace every form in `text` with `[secret NAME]`, counting each into `redacted`. #[must_use] pub fn scrub(&self, text: &str, redacted: &mut u32) -> String { diff --git a/core/crates/ployz-core/src/sealed_record.rs b/core/crates/ployz-core/src/sealed_record.rs new file mode 100644 index 0000000000..263b8875b0 --- /dev/null +++ b/core/crates/ployz-core/src/sealed_record.rs @@ -0,0 +1,216 @@ +use std::{collections::BTreeMap, fmt}; + +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use thiserror::Error; +use ts_rs::TS; +use zeroize::Zeroizing; + +use crate::{EnvValue, RequestedServiceSpec, SealedIdentity, scrub::SealedValues}; + +/// One Service's Sealed values, still sealed, with who they were sealed for. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] +pub struct SealedRecord { + identity: SealedIdentity, + values: BTreeMap, +} + +/// A Service reads a Sealed value but its Deploy names no Environment to open it in. +#[derive(Debug, Error)] +#[error("a sealed value needs the Deploy's Environment")] +pub struct MissingSealedIdentity; + +impl SealedRecord { + #[must_use] + pub const fn new(identity: SealedIdentity, values: BTreeMap) -> Self { + Self { identity, values } + } + + /// The Sealed values `service` reads, or `None` when it reads none. + /// + /// # Errors + /// Fails when `service` reads one without a Sealed identity. + pub fn of(service: &RequestedServiceSpec) -> Result, MissingSealedIdentity> { + let values: BTreeMap = service + .container + .environment + .iter() + .filter(|(_, value)| value.is_sealed()) + .map(|(name, value)| (name.clone(), value.clone())) + .collect(); + if values.is_empty() { + return Ok(None); + } + let identity = service + .container + .sealed_identity + .clone() + .ok_or(MissingSealedIdentity)?; + Ok(Some(Self { identity, values })) + } + + #[must_use] + pub const fn identity(&self) -> &SealedIdentity { + &self.identity + } + + #[must_use] + pub const fn values(&self) -> &BTreeMap { + &self.values + } + + pub fn keys(&self) -> impl Iterator { + self.values.keys().map(String::as_str) + } +} + +/// A record's values opened for one Build: each whole by name, and each Sealed part a +/// Template joined, which a scrub hides too. Its Debug prints names only, and it +/// serializes only as the body a Build Grant's holder reads. +pub struct OpenedSealed { + values: BTreeMap>, + parts: Vec<(String, Zeroizing)>, +} + +/// The holder's Build reads a Sealed value its grant doesn't open. +#[derive(Debug, Error)] +#[error("the Build Grant does not open {0}")] +pub struct MissingSealed(pub String); + +impl OpenedSealed { + #[must_use] + pub fn new(values: BTreeMap, parts: Vec<(String, String)>) -> Self { + Self { + values: values + .into_iter() + .map(|(name, value)| (name, Zeroizing::new(value))) + .collect(), + parts: parts + .into_iter() + .map(|(name, value)| (name, Zeroizing::new(value))) + .collect(), + } + } + + #[must_use] + pub fn is_empty(&self) -> bool { + self.values.is_empty() + } + + #[must_use] + pub fn get(&self, name: &str) -> Option<&str> { + self.values.get(name).map(|value| value.as_str()) + } + + pub fn values(&self) -> impl Iterator { + self.values + .iter() + .map(|(name, value)| (name.as_str(), value.as_str())) + } + + /// Every form a scrub hides: each value whole and each Sealed part. + #[must_use] + pub fn secrets(&self) -> SealedValues { + self.values + .iter() + .chain(self.parts.iter().map(|(name, value)| (name, value))) + .map(|(name, value)| (name.clone(), value.as_str().to_owned())) + .collect() + } + + /// Refuse a set that lacks one of `names`, the values a Build reads. + /// + /// # Errors + /// Names the first value missing. + pub fn covering<'name>( + self, + mut names: impl Iterator, + ) -> Result { + match names.find(|name| !self.values.contains_key(*name)) { + Some(name) => Err(MissingSealed(name.to_owned())), + None => Ok(self), + } + } +} + +impl fmt::Debug for OpenedSealed { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.values.keys()).finish() + } +} + +#[derive(Serialize)] +struct WireRef<'a> { + values: BTreeMap<&'a str, &'a str>, + parts: Vec<(&'a str, &'a str)>, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct Wire { + values: BTreeMap, + parts: Vec<(String, String)>, +} + +impl Serialize for OpenedSealed { + fn serialize(&self, serializer: S) -> Result { + WireRef { + values: self.values().collect(), + parts: self + .parts + .iter() + .map(|(name, value)| (name.as_str(), value.as_str())) + .collect(), + } + .serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for OpenedSealed { + fn deserialize>(deserializer: D) -> Result { + let Wire { values, parts } = Wire::deserialize(deserializer)?; + Ok(Self::new(values, parts)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn opened() -> OpenedSealed { + OpenedSealed::new( + BTreeMap::from([ + ("NPM_TOKEN".into(), "marker-token".into()), + ("URL".into(), "https://u:marker-part@host".into()), + ]), + vec![("PASSWORD".into(), "marker-part".into())], + ) + } + + #[test] + fn debug_names_values_and_shows_none() { + let debug = format!("{:?}", opened()); + assert_eq!(debug, r#"["NPM_TOKEN", "URL"]"#); + assert!(!debug.contains("marker")); + } + + #[test] + fn the_wire_body_round_trips_every_form() { + let body = serde_json::to_string(&opened()).unwrap(); + let back: OpenedSealed = serde_json::from_str(&body).unwrap(); + assert_eq!(back.get("NPM_TOKEN"), Some("marker-token")); + assert_eq!( + format!("{:?}", back.secrets()), + format!("{:?}", opened().secrets()) + ); + assert_eq!(serde_json::to_string(&back).unwrap(), body); + } + + #[test] + fn a_set_covers_only_the_names_it_holds() { + assert!(opened().covering(["NPM_TOKEN"].into_iter()).is_ok()); + let missing = opened() + .covering(["NPM_TOKEN", "OTHER"].into_iter()) + .unwrap_err(); + assert_eq!(missing.to_string(), "the Build Grant does not open OTHER"); + } +} diff --git a/core/crates/ployz-core/tests/capabilities.rs b/core/crates/ployz-core/tests/capabilities.rs index a6200acf9e..df6d8620e2 100644 --- a/core/crates/ployz-core/tests/capabilities.rs +++ b/core/crates/ployz-core/tests/capabilities.rs @@ -1,23 +1,23 @@ use ployz_core::{ - BUILD_SEALED_CAPABILITY, CERTIFICATE_POLICY_CAPABILITY, CapabilityAdvertisement, - CapabilityName, DESCRIBE_CONTRACT_CAPABILITY, ENROLL_FOR_RECOVERY_CAPABILITY, - ENROLL_MACHINE_CAPABILITY, ENSURE_CONTAINER_CAPABILITY, ENSURE_IMAGE_INGEST_CAPABILITY, - ENV_SEALED_CAPABILITY, EXEC_CONTAINER_CAPABILITY, FORGET_MACHINE_CAPABILITY, - GENERATE_SEALING_KEY_CAPABILITY, INSPECT_BUILD_GRANT_CAPABILITY, INSPECT_CONTAINER_CAPABILITY, - INSPECT_MACHINE_CAPABILITY, INSPECT_STORAGE_CAPABILITY, INSPECT_VOLUME_COPY_CAPABILITY, - INSTALL_SEALING_KEY_CAPABILITY, LEAVE_MACHINE_CAPABILITY, LIST_CONTAINERS_CAPABILITY, - LIST_IMAGES_CAPABILITY, LIST_MACHINES_CAPABILITY, LIST_VOLUMES_CAPABILITY, - LOG_HISTORY_CAPABILITY, MACHINE_STORAGE_OBSERVATION_CAPABILITY, MANAGEMENT_SCOPE_CAPABILITY, - METRIC_HISTORY_CAPABILITY, MINT_BUILD_GRANT_CAPABILITY, PREPARE_VOLUMES_CAPABILITY, - PRUNE_IMAGES_CAPABILITY, PUBLISH_CERTIFICATE_MATERIAL_CAPABILITY, PULL_IMAGE_CAPABILITY, - PULL_IMAGE_FROM_MACHINE_CAPABILITY, READ_ROUTED_ADDRESSES_CAPABILITY, READ_ROUTES_CAPABILITY, - RECOVER_SEALING_KEY_CAPABILITY, REGISTER_MACHINE_CAPABILITY, REMOVE_VOLUME_CAPABILITY, - REQUEST_MACHINE_UPGRADE_CAPABILITY, RETAINED_ENV_CAPABILITY, RUNTIME_WATCH_CAPABILITY, Rpc, - SEED_VOLUME_CAPABILITY, SELECT_ENDPOINT_CAPABILITY, SET_DEVICE_CAPABILITY, - SET_MANAGEMENT_CLIENT_CAPABILITY, SET_PUSH_TARGET_CAPABILITY, SET_VOLUME_COPY_CAPABILITY, - SET_VOLUME_SIZE_CAPABILITY, SHARE_SEALING_KEY_CAPABILITY, TAIL_LOGS_CAPABILITY, - UPDATE_CONTAINER_LIMITS_CAPABILITY, UPDATE_MACHINE_CAPABILITY, VOLUME_FLOOR_CAPABILITY, - VOLUME_SHRINK_CAPABILITY, WRAP_SEALING_KEY_CAPABILITY, op, + BUILD_GRANT_HOLDER_CAPABILITY, BUILD_SEALED_CAPABILITY, CERTIFICATE_POLICY_CAPABILITY, + CapabilityAdvertisement, CapabilityName, DESCRIBE_CONTRACT_CAPABILITY, + ENROLL_FOR_RECOVERY_CAPABILITY, ENROLL_MACHINE_CAPABILITY, ENSURE_CONTAINER_CAPABILITY, + ENSURE_IMAGE_INGEST_CAPABILITY, ENV_SEALED_CAPABILITY, EXEC_CONTAINER_CAPABILITY, + FORGET_MACHINE_CAPABILITY, GENERATE_SEALING_KEY_CAPABILITY, INSPECT_BUILD_GRANT_CAPABILITY, + INSPECT_CONTAINER_CAPABILITY, INSPECT_MACHINE_CAPABILITY, INSPECT_STORAGE_CAPABILITY, + INSPECT_VOLUME_COPY_CAPABILITY, INSTALL_SEALING_KEY_CAPABILITY, LEAVE_MACHINE_CAPABILITY, + LIST_CONTAINERS_CAPABILITY, LIST_IMAGES_CAPABILITY, LIST_MACHINES_CAPABILITY, + LIST_VOLUMES_CAPABILITY, LOG_HISTORY_CAPABILITY, MACHINE_STORAGE_OBSERVATION_CAPABILITY, + MANAGEMENT_SCOPE_CAPABILITY, METRIC_HISTORY_CAPABILITY, MINT_BUILD_GRANT_CAPABILITY, + PREPARE_VOLUMES_CAPABILITY, PRUNE_IMAGES_CAPABILITY, PUBLISH_CERTIFICATE_MATERIAL_CAPABILITY, + PULL_IMAGE_CAPABILITY, PULL_IMAGE_FROM_MACHINE_CAPABILITY, READ_ROUTED_ADDRESSES_CAPABILITY, + READ_ROUTES_CAPABILITY, RECOVER_SEALING_KEY_CAPABILITY, REGISTER_MACHINE_CAPABILITY, + REMOVE_VOLUME_CAPABILITY, REQUEST_MACHINE_UPGRADE_CAPABILITY, RETAINED_ENV_CAPABILITY, + RUNTIME_WATCH_CAPABILITY, Rpc, SEED_VOLUME_CAPABILITY, SELECT_ENDPOINT_CAPABILITY, + SET_DEVICE_CAPABILITY, SET_MANAGEMENT_CLIENT_CAPABILITY, SET_PUSH_TARGET_CAPABILITY, + SET_VOLUME_COPY_CAPABILITY, SET_VOLUME_SIZE_CAPABILITY, SHARE_SEALING_KEY_CAPABILITY, + TAIL_LOGS_CAPABILITY, UPDATE_CONTAINER_LIMITS_CAPABILITY, UPDATE_MACHINE_CAPABILITY, + VOLUME_FLOOR_CAPABILITY, VOLUME_SHRINK_CAPABILITY, WRAP_SEALING_KEY_CAPABILITY, op, }; /// Capability constants are generated from the catalog, so a typo would stay @@ -63,6 +63,7 @@ fn catalogued_capabilities_keep_stable_spellings() { (MANAGEMENT_SCOPE_CAPABILITY, "ployz.management.scope.v1"), (ENV_SEALED_CAPABILITY, "ployz.env.sealed.v1"), (BUILD_SEALED_CAPABILITY, "ployz.build.sealed.v1"), + (BUILD_GRANT_HOLDER_CAPABILITY, "ployz.build.grant.holder.v1"), (LIST_CONTAINERS_CAPABILITY, "ployz.container.list.v1"), (INSPECT_CONTAINER_CAPABILITY, "ployz.container.inspect.v1"), (ENSURE_CONTAINER_CAPABILITY, "ployz.container.ensure.v1"), @@ -160,6 +161,7 @@ fn advertised_capability_groups_match_the_frozen_catalog() { "ployz.management.scope.v1", "ployz.env.sealed.v1", "ployz.build.sealed.v1", + "ployz.build.grant.holder.v1", ] ); assert_eq!( @@ -362,6 +364,10 @@ const ABOVE_FLOOR: &[(&str, &str)] = &[ "ployz.build.sealed.v1", "gated: Builder selection for a Build that reads a Sealed value considers only Machines advertising it, and refuses naming each Machine that lacks it", ), + ( + "ployz.build.grant.holder.v1", + "gated: a GitHub check-in mints for the runner's key only on a Machine advertising it; otherwise it mints the old grant, and a Build that reads a Sealed value goes to its next Builder", + ), ( "ployz.sealing.generate.v1", "gated: the Sealing Key backfill reads each Machine's contract first and never picks a generator that lacks it", diff --git a/core/crates/ployz-sdk/index.d.ts b/core/crates/ployz-sdk/index.d.ts index 5d991a2b30..2cc5c4baf4 100644 --- a/core/crates/ployz-sdk/index.d.ts +++ b/core/crates/ployz-sdk/index.d.ts @@ -313,7 +313,7 @@ export interface ConfigStore extends Omit { read(organization: string, query: Q, trusted?: ConfigTrusted): Promise>; } export type GithubStart = { kind: "reused" } | { kind: "dispatch"; runner: string } | { kind: "skipped"; message: string }; -export type GithubCheckIn = { grant: string; commit: string; fingerprint: string; ployzVersion: string; deployment: PreparationInput["deployment"] }; +export type GithubCheckIn = ({ grant: string } | { machine: string }) & { commit: string; fingerprint: string; ployzVersion: string; deployment: PreparationInput["deployment"] }; export type GithubFinish = "waiting" | { ended: BuildStatus }; /** Calls run off the JavaScript thread, a few at once; a slow or queued call rejects `unavailable`. */ export declare function openConfigStore(url: string, sealingSecret: string): Promise; diff --git a/core/crates/ployz-sdk/src/config_store.rs b/core/crates/ployz-sdk/src/config_store.rs index 8bca3e14cc..b7548e9ee7 100644 --- a/core/crates/ployz-sdk/src/config_store.rs +++ b/core/crates/ployz-sdk/src/config_store.rs @@ -442,11 +442,11 @@ impl ConfigStore { .await } - /// A build handed to GitHub (`GithubBuild`). + /// Where a build handed to GitHub is (`GithubHandOff`). /// /// # Errors - /// Returns `not_found` unless GitHub holds or held it. - #[napi(ts_return_type = "Promise")] + /// Returns `not_found` unless GitHub holds it, held it, or passed it on. + #[napi(ts_return_type = "Promise")] pub async fn github_build(&self, build: String) -> Result { let build = github_id(&build)?; self.run(move |store| store.github_build(&build)).await @@ -465,23 +465,39 @@ impl ConfigStore { .await } - /// A runner's check-in with its verified OIDC `claims` (`GithubClaims`): its Build - /// Grant and build inputs, secrets included. Only the runner may receive them. + /// A runner's check-in with its verified OIDC `claims` (`GithubClaims`) and the + /// public key it holds its Build Grant with, if any: how it pushes and its build + /// inputs, secrets included. Only the runner may receive them. /// /// # Errors - /// Returns `unauthenticated` for another repository, workflow or run, `conflict` - /// once it checked in or is no longer wanted, or the Machine's refusal. + /// Returns `invalid_argument` for a holder that is not 64 lowercase hex, + /// `unauthenticated` for another repository, workflow or run, `conflict` once it + /// checked in, is no longer wanted or GitHub skipped it, or the Machine's refusal. #[napi(ts_return_type = "Promise")] pub async fn github_check_in( &self, build: String, #[napi(ts_arg_type = "GithubClaims")] claims: serde_json::Value, + #[napi(ts_arg_type = "string | null")] holder: Option, #[napi(ts_arg_type = "Connection[]")] connections: serde_json::Value, ) -> Result { let (build, claims) = (github_id(&build)?, claims_of(claims)?); + let holder = holder + .map(|holder| { + ployz_core::BuildGrantId::parse(holder) + .map_err(|_| invalid_argument("Expected a holder of 64 lowercase hex")) + }) + .transpose()?; let connections = crate::parse_connections(connections)?; to_json( - ployz::sdk::github_check_in(Arc::clone(&self.store), build, claims, connections).await, + ployz::sdk::github_check_in( + Arc::clone(&self.store), + build, + claims, + holder, + connections, + ) + .await, ) } diff --git a/core/crates/ployz-store/src/build.rs b/core/crates/ployz-store/src/build.rs index e3dbdc0481..e017ad2e0c 100644 --- a/core/crates/ployz-store/src/build.rs +++ b/core/crates/ployz-store/src/build.rs @@ -18,9 +18,6 @@ use crate::error; use crate::id::DeploymentId; use crate::storage::{Param, Tx, name_of}; -/// Why a Build that reads a Sealed value runs only on a Server Builder. -pub const SEALED_BUILDS_RUN_ON_SERVER: &str = "Builds that read a sealed value run on your server."; - /// The most log a build keeps; older output goes first. const LOG_LIMIT: usize = 256 * 1024; @@ -47,11 +44,6 @@ pub struct GitSource { pub builders: Vec, /// The Server the servers try first, when it prefers one. pub preferred_machine: Option, - /// The first of its variables that reads a sealed value, when one does: its - /// build runs on a Server. - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub sealed: Option, /// Where its build is; none until pinned. pub status: Option, /// Why its build failed, or why the last Builder skipped it. @@ -135,13 +127,9 @@ pub struct BuildLogView { } /// The Git Services Deployment `id` builds, each with its pin, if any. -pub(crate) fn sources( - tx: &mut dyn Tx, - id: &DeploymentId, - sealing: &crate::SealingKey, -) -> Result, RpcError> { +pub(crate) fn sources(tx: &mut dyn Tx, id: &DeploymentId) -> Result, RpcError> { let stored = deployment::load(tx, id)?; - sources_of(tx, &stored, sealing) + sources_of(tx, &stored) } /// Pin the commits of `commits` (by runtime Service name) that aren't pinned yet. A @@ -396,80 +384,26 @@ pub(crate) fn uploads_of( .collect()) } -/// The Git Services `stored` builds, with the Builders each tries and the sealed -/// value it reads, if any. -pub(crate) fn sources_of( - tx: &mut dyn Tx, - stored: &Stored, - sealing: &crate::SealingKey, -) -> Result, RpcError> { +/// The Git Services `stored` builds, with the Builders each tries. +pub(crate) fn sources_of(tx: &mut dyn Tx, stored: &Stored) -> Result, RpcError> { let saved = deployment::saved_at(tx, &stored.summary.environment_id, stored.summary.saved)?; - // A value reaches a Service Sealed only when its ciphertext is for this - // Environment, and such a value is one of its own variables: a Branch reads a - // parent's as Not set. So an Environment without one resolves nothing here. - let sealed = if reads_sealed(&saved) { - match deployment::sealed_reads(tx, stored, &saved, sealing) { - Ok(sealed) => sealed, - // Claim refuses this revision and records why. Until then its builds - // still list, each reading its own sealed values. - Err(refused) if refused.code == ployz_core::RpcErrorCode::InvalidArgument => { - own_sealed(&saved) - } - Err(error) => return Err(error), - } - } else { - BTreeMap::new() - }; - routed(tx, stored, &saved, sealed) -} - -/// The first variable of each Service of `saved`, by key and by Service ID, that -/// holds a value the browser sealed. -fn own_sealed(saved: &ployz_core::config::SavedEnvironmentIntent) -> BTreeMap { - saved - .services - .iter() - .filter_map(|service| { - let key = service - .variables - .iter() - .filter(|variable| crate::client_sealed::browser_sealed(variable)) - .map(|variable| &variable.key) - .min()?; - Some((service.id.clone(), key.clone())) - }) - .collect() -} - -/// Whether any variable of `saved` holds a value the browser sealed: the fact the -/// dashboard's Preferred Builder line reads. -fn reads_sealed(saved: &ployz_core::config::SavedEnvironmentIntent) -> bool { - saved - .services - .iter() - .flat_map(|service| &service.variables) - .any(crate::client_sealed::browser_sealed) + routed(tx, stored, &saved) } -/// The Git Services `stored` builds from `saved`, each with the Builders it tries, -/// given `sealed`: the first variable of each Service, by Service ID, that reads a -/// sealed value. +/// The Git Services `stored` builds from `saved`, each with the Builders it tries. pub(crate) fn routed( tx: &mut dyn Tx, stored: &Stored, saved: &ployz_core::config::SavedEnvironmentIntent, - mut sealed: BTreeMap, ) -> Result, RpcError> { let organization = organization(tx, &stored.summary.id)?; let mut sources = Vec::new(); for (id, mut source) in git_sources(tx, stored, saved)? { - source.sealed = sealed.remove(&id); (source.builders, source.preferred_machine) = builders::walk( tx, organization.as_str(), &stored.summary.environment_id, &id, - source.sealed.is_some(), )?; sources.push(source); } @@ -541,7 +475,6 @@ fn source_of( commit: row.and_then(|row| row.commit.clone()), builders: Vec::new(), preferred_machine: None, - sealed: None, status: row.map(|row| row.status), message: row.and_then(|row| row.message.clone()), }) @@ -678,6 +611,16 @@ pub struct GithubBuild { pub ended: Option, } +/// Where a build handed to GitHub is. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] +#[serde(rename_all = "snake_case")] +pub enum GithubHandOff { + /// GitHub holds it, or held it until it ended. + Held(Box), + /// GitHub passed it to the next Builder, and its run holds it no more. + PassedOn { why: String }, +} + /// The claims of a GitHub Actions OIDC token Cloud verified, as GitHub sends them. #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] pub struct GithubClaims { @@ -777,7 +720,8 @@ impl BuildRow { } /// Hand a pinned, unstarted build to GitHub run `run`. Handing it the same run again -/// changes nothing; any other hand-off is `conflict`, and Cloud cancels its run. +/// changes nothing; any other hand-off, or one after GitHub passed it on, is +/// `conflict`, and Cloud cancels its run. pub(crate) fn github_dispatched( tx: &mut dyn Tx, id: &GithubBuildId, @@ -790,7 +734,7 @@ pub(crate) fn github_dispatched( } if !stored.summary.status.in_flight() || row.status != BuildStatus::Pending - || row.github.is_some() + || row.github_skipped.is_some() { return Err(no_longer_wanted()); } @@ -806,14 +750,17 @@ pub(crate) fn github_dispatched( received: 0, ended: None, }; - save_github(tx, id, BuildStatus::Building, None, &log, Some(&github))?; - skip(tx, id, None) + save_github(tx, id, BuildStatus::Building, None, &log, Some(&github)) } -pub(crate) fn github_build(tx: &mut dyn Tx, id: &GithubBuildId) -> Result { +pub(crate) fn github_build(tx: &mut dyn Tx, id: &GithubBuildId) -> Result { let stored = deployment::load(tx, &id.deployment)?; - github_row(tx, &stored, id)? - .github(id) + let row = github_row(tx, &stored, id)?; + if let Some(build) = row.github(id) { + return Ok(GithubHandOff::Held(Box::new(build))); + } + row.github_skipped + .map(|why| GithubHandOff::PassedOn { why }) .ok_or_else(|| error::not_found("No GitHub build has this id", json!({}))) } @@ -855,46 +802,60 @@ pub(crate) fn github_authorize( Ok(build) } -/// What GitHub build `id` builds: its Deployment's lowering input with secrets -/// unsealed and every value sealed in the browser left out, its pinned commit, and -/// its Service's latest receipt. `conflict` when its Service reads a sealed value: -/// only a Server builds it. In-process only. +/// What a GitHub build builds. +#[derive(Clone, Debug, PartialEq)] +pub struct GithubInput { + /// The Deployment's lowering input with secrets unsealed, and every value sealed + /// in the browser left out but the built Service's. + pub input: serde_json::Value, + pub commit: CommitSha, + /// The Service's latest receipt. + pub receipt: Option, +} + +/// What GitHub build `id` builds. In-process only. pub(crate) fn github_input( tx: &mut dyn Tx, id: &GithubBuildId, sealing: &crate::SealingKey, -) -> Result<(serde_json::Value, CommitSha, Option), RpcError> { +) -> Result { let stored = deployment::load(tx, &id.deployment)?; let row = github_row(tx, &stored, id)?; let commit = row .commit .ok_or_else(|| error::corrupt("GitHub build commit"))?; let saved = deployment::held_revision(tx, &stored)?; - let deployment::Lowered { - mut input, sealed, .. - } = deployment::lowered(tx, &stored, &saved, sealing)?; - if !sealed.is_empty() { + let deployment::Lowered { mut input, .. } = deployment::lowered(tx, &stored, &saved, sealing)?; + let sealed = input + .snapshots + .iter() + .flat_map(|snapshot| snapshot.resolved_env.values()) + .any(ployz_core::EnvValue::is_sealed); + let built = if sealed { let built = saved .services .iter() .find(|service| service.config.private_dns == id.service) .ok_or_else(|| error::corrupt("GitHub build Service"))?; - if let Some(sealed) = sealed.get(&built.id) { - return Err(error::conflict( - format!("{sealed} is sealed. {SEALED_BUILDS_RUN_ON_SERVER}"), - json!({ "service": id.service, "variable": sealed }), - )); - } - } + Some(&built.id) + } else { + None + }; for snapshot in &mut input.snapshots { - snapshot.resolved_env.retain(|_, value| !value.is_sealed()); + if snapshot.service_id.as_ref() != built { + snapshot.resolved_env.retain(|_, value| !value.is_sealed()); + } } let input = deployment::json_value(&input); // The latest receipt of the Service, a cache hint; its own when it has one. let receipt = deployment::receipts(tx, &stored.summary.environment_id)? .remove(&id.service) .and_then(|receipts| receipts.into_iter().next()); - Ok((input, commit, receipt)) + Ok(GithubInput { + input, + commit, + receipt, + }) } /// Run `run_id` checked in and got `grant`: the build starts. Accepted once, while @@ -978,8 +939,9 @@ pub(crate) fn github_report( } /// End GitHub build `id`: its run `run_id`'s, or with none, before GitHub took it -/// (an earlier image served it, or GitHub couldn't take it). Ending it again, or a -/// run that no longer holds it, is `conflict`. +/// (an earlier image served it, or GitHub couldn't take it). Ending it again, a run +/// that no longer holds it, or ending it with none after GitHub passed it on, is +/// `conflict`. pub(crate) fn github_end( tx: &mut dyn Tx, id: &GithubBuildId, @@ -993,7 +955,7 @@ pub(crate) fn github_end( .building_on(run_id) .is_some_and(|github| github.grant.is_none()), (Some(run_id), _) => row.building_on(run_id).is_some(), - (None, _) => row.status == BuildStatus::Pending && row.github.is_none(), + (None, _) => row.status == BuildStatus::Pending && row.github_skipped.is_none(), }; if !holds { return Err(error::conflict("This build already ended", json!({}))); @@ -1017,23 +979,21 @@ pub(crate) fn github_end( row.github.as_ref(), ) } - // The next Builder starts it afresh. + // The next Builder starts it afresh: the run holds it no more. GithubEnd::Skipped { message } | GithubEnd::Unstarted { message } => { append(&mut log, &format!("GitHub skipped: {message}\n")); (BuildStatus::Pending, Some(trimmed(message)), None) } }; save_github(tx, id, status, message.as_deref(), &log, github)?; - let skipped = match end { - GithubEnd::Skipped { .. } | GithubEnd::Unstarted { .. } => message.as_deref(), - GithubEnd::Built { .. } | GithubEnd::Failed { .. } => None, - }; - skip(tx, id, skipped)?; + if let (GithubEnd::Skipped { .. } | GithubEnd::Unstarted { .. }, Some(why)) = (end, &message) { + skip(tx, id, why)?; + } Ok(status) } -/// Record why GitHub passed `id` on, or with `None` that GitHub has it again. -fn skip(tx: &mut dyn Tx, id: &GithubBuildId, why: Option<&str>) -> Result<(), RpcError> { +/// Record why GitHub passed `id` on. +fn skip(tx: &mut dyn Tx, id: &GithubBuildId, why: &str) -> Result<(), RpcError> { tx.execute( "UPDATE config_build SET github_skipped = ?3 WHERE deployment_id = ?1 AND service = ?2", &[ diff --git a/core/crates/ployz-store/src/builders.rs b/core/crates/ployz-store/src/builders.rs index 3772239cbf..7f3aa6e646 100644 --- a/core/crates/ployz-store/src/builders.rs +++ b/core/crates/ployz-store/src/builders.rs @@ -150,13 +150,12 @@ impl Preferred { } /// The Builders a Git build of `service_id` tries, in turn, and the Server the -/// servers try first, if one is preferred. A `sealed` build never tries GitHub. +/// servers try first, if one is preferred. pub(crate) fn walk( tx: &mut dyn Tx, organization: &str, environment: &EnvironmentId, service_id: &str, - sealed: bool, ) -> Result<(Vec, Option), RpcError> { let order = chosen(tx, organization)?.unwrap_or(BuildOrder::AUTO); let preferred = crate::policy::load(tx, environment, service_id)? @@ -173,12 +172,5 @@ pub(crate) fn walk( .iter() .copied() .filter(|builder| Some(*builder) != first); - Ok(( - first - .into_iter() - .chain(rest) - .filter(|builder| !(sealed && *builder == Builder::Github)) - .collect(), - machine, - )) + Ok((first.into_iter().chain(rest).collect(), machine)) } diff --git a/core/crates/ployz-store/src/deployment/lifecycle.rs b/core/crates/ployz-store/src/deployment/lifecycle.rs index c5dfa23317..520726bb82 100644 --- a/core/crates/ployz-store/src/deployment/lifecycle.rs +++ b/core/crates/ployz-store/src/deployment/lifecycle.rs @@ -1215,7 +1215,6 @@ pub(crate) fn claim( input, mut intent, warnings, - sealed, .. } = match lowered(tx, &stored, &saved, sealing) { Ok(lowered) => lowered, @@ -1238,7 +1237,7 @@ pub(crate) fn claim( } Err(error) => return Err(error), }; - let sources = build::routed(tx, &stored, &saved, sealed)?; + let sources = build::routed(tx, &stored, &saved)?; if starting { let taken_over = tx.execute( "UPDATE config_deployment SET status = 'unknown' \ @@ -1377,24 +1376,6 @@ pub(crate) fn lowered( ) } -/// The first variable of each Service of `saved`, by Service ID, whose value reads a -/// value sealed in the browser, as `stored` lowers it. -pub(crate) fn sealed_reads( - tx: &mut dyn Tx, - stored: &Stored, - saved: &SavedEnvironmentIntent, - sealing: &SealingKey, -) -> Result, RpcError> { - let branch = crate::branch::lowering(tx, &stored.summary.environment_id, saved)?; - super::lowering::first_sealed( - &stored.summary.environment_id, - saved, - stored.cluster_domain.as_ref(), - &branch, - sealing, - ) -} - /// Cancel a Deployment of `who`'s Organization. A queued one, or one waiting for CI, /// never runs; a running one reads `cancelling` until its runner stops it and records /// what ran. Cancelling again changes nothing. diff --git a/core/crates/ployz-store/src/deployment/lowering.rs b/core/crates/ployz-store/src/deployment/lowering.rs index ad8446448d..a435069d6e 100644 --- a/core/crates/ployz-store/src/deployment/lowering.rs +++ b/core/crates/ployz-store/src/deployment/lowering.rs @@ -603,10 +603,6 @@ pub(crate) struct Lowered { /// left empty. Only lowering with the sealing key sees every value, so only it /// warns of those. pub(crate) warnings: Vec, - /// The first variable of each Service, by Service ID, whose value reads a value - /// sealed in the browser: its build runs on a Server. Names only. Empty without - /// the sealing key, which leaves such a variable out. - pub(crate) sealed: BTreeMap, } /// `saved` compiled, and its variables and Config files resolved as [`lower`] @@ -638,35 +634,6 @@ fn resolve( Ok((compiled, resolved)) } -/// The first variable of each Service of `saved`, by Service ID, whose value reads -/// a value sealed in the browser, resolved as [`lower`] resolves it. Names only: -/// no value leaves here. Refuses what [`lower`] refuses. -pub(super) fn first_sealed( - environment: &EnvironmentId, - saved: &SavedEnvironmentIntent, - cluster_domain: Option<&Hostname>, - branch: &crate::branch::Lowering, - sealing: &SealingKey, -) -> Result, RpcError> { - let (_, resolved) = resolve(environment, saved, cluster_domain, branch, Some(sealing))?; - Ok(sealed_names(&resolved)) -} - -/// The first variable of each Service of `resolved`, by Service ID, whose value -/// reads a value sealed in the browser. -fn sealed_names(resolved: &variables::Resolved) -> BTreeMap { - resolved - .env - .iter() - .filter_map(|(service, env)| { - let name = env - .iter() - .find_map(|(name, value)| value.is_sealed().then_some(name))?; - Some((service.clone(), name.clone())) - }) - .collect() -} - /// Lower Saved revision `saved` to the Deploy Intent of a Deployment that /// `ships` into its Namespace, with generated domains under the /// Cluster Domain. Variables and Config files resolve here, a Branch's Live Node @@ -686,7 +653,6 @@ pub(super) fn lower( ) -> Result { let (unseal, organization) = unseal.unzip(); let (compiled, mut resolved) = resolve(environment, saved, cluster_domain, branch, unseal)?; - let sealed = sealed_names(&resolved); let (narrowed, services, removals, prune): (bool, &[ServiceName], _, _) = match ships { Ships::All { removals, prune } => ( false, @@ -992,7 +958,6 @@ pub(super) fn lower( shipped, intent, warnings, - sealed, }) } diff --git a/core/crates/ployz-store/src/lib.rs b/core/crates/ployz-store/src/lib.rs index c25b17ea44..c9486f89a7 100644 --- a/core/crates/ployz-store/src/lib.rs +++ b/core/crates/ployz-store/src/lib.rs @@ -63,8 +63,8 @@ pub use branch::{ }; pub use build::{ BuildLogQuery, BuildLogView, BuildReport, BuildStatus, BuildView, GitSource, GithubBuild, - GithubBuildId, GithubClaims, GithubEnd, GithubGrant, GithubReport, GithubRun, RunEnd, - SEALED_BUILDS_RUN_ON_SERVER, + GithubBuildId, GithubClaims, GithubEnd, GithubGrant, GithubHandOff, GithubInput, GithubReport, + GithubRun, RunEnd, }; pub use builders::{BuildOrder, BuildOrderQuery, BuildOrderView, Builder, SetBuildOrder}; pub use client_sealed::SecretValue; @@ -356,8 +356,7 @@ impl ConfigStore { /// # Errors /// Returns `not_found` for an unknown Deployment, or a storage error. pub fn sources(&self, deployment: &DeploymentId) -> Result, RpcError> { - self.storage - .read(|tx| build::sources(tx, deployment, &self.sealing)) + self.storage.read(|tx| build::sources(tx, deployment)) } /// Pin the commit each Git Service of Deployment `deployment` builds, by runtime @@ -376,7 +375,7 @@ impl ConfigStore { ) -> Result, RpcError> { self.storage.write(|tx| { build::pin(tx, deployment, commits)?; - build::sources(tx, deployment, &self.sealing) + build::sources(tx, deployment) }) } @@ -468,8 +467,9 @@ impl ConfigStore { /// A Git build handed to GitHub. In-process only. /// /// # Errors - /// Returns `not_found` unless GitHub holds or held it, or a storage error. - pub fn github_build(&self, id: &GithubBuildId) -> Result { + /// Returns `not_found` unless GitHub holds it, held it, or passed it on, or a + /// storage error. + pub fn github_build(&self, id: &GithubBuildId) -> Result { self.storage.read(|tx| build::github_build(tx, id)) } @@ -488,16 +488,12 @@ impl ConfigStore { .read(|tx| build::github_authorize(tx, id, claims)) } - /// What GitHub build `id` builds: the Deployment's lowering input with its - /// secrets unsealed, the pinned commit, and the Service's latest receipt. - /// In-process only: only the authorized runner receives it. + /// What GitHub build `id` builds. In-process only: only the authorized runner + /// receives it. /// /// # Errors /// Returns `not_found` for an unknown build, or a storage error. - pub fn github_input( - &self, - id: &GithubBuildId, - ) -> Result<(serde_json::Value, CommitSha, Option), RpcError> { + pub fn github_input(&self, id: &GithubBuildId) -> Result { self.storage .read(|tx| build::github_input(tx, id, &self.sealing)) } diff --git a/core/crates/ployz-store/src/settings/query.rs b/core/crates/ployz-store/src/settings/query.rs index 8ab10ad64d..b924b487ee 100644 --- a/core/crates/ployz-store/src/settings/query.rs +++ b/core/crates/ployz-store/src/settings/query.rs @@ -47,10 +47,6 @@ pub struct EnvironmentView { #[serde(default, skip_serializing_if = "Vec::is_empty")] #[ts(as = "Option>", optional)] pub never_synced: Vec, - /// Every variable row of the Services read whose value the browser sealed. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - #[ts(as = "Option>", optional)] - pub sealed: Vec, } /// One Setting's current Working State value. @@ -104,16 +100,7 @@ pub(crate) fn environment( _ => crate::branch::Own::of(tx, &environment)?, }; let mut settings = Vec::new(); - let mut sealed = Vec::new(); for service in services { - for variable in &service.variables { - if crate::client_sealed::browser_sealed(variable) { - sealed.push(crate::branch::row_id( - &service.lineage_id, - &format!("variables.{}", variable.key), - )?); - } - } let name = ServiceName::parse(service.slug.as_str()) .map_err(|_| crate::error::corrupt("Service name"))?; let mut row = |target: Target, value: Value, default: Value, apply: Apply| { @@ -230,7 +217,6 @@ pub(crate) fn environment( environment: environment.summary, settings, values, - sealed, }) } diff --git a/core/crates/ployz-store/tests/git.rs b/core/crates/ployz-store/tests/git.rs index 225fdee7e6..50142be38c 100644 --- a/core/crates/ployz-store/tests/git.rs +++ b/core/crates/ployz-store/tests/git.rs @@ -15,8 +15,8 @@ use ployz_store::{ SettingPath, SourceKind, Trusted, View, Written, }; use ployz_store::{ - BuildOrder, BuildOrderQuery, Builder, GithubBuildId, GithubClaims, GithubEnd, GithubGrant, - GithubReport, GithubRun, SetBuildOrder, + BuildOrder, BuildOrderQuery, Builder, GithubBuild, GithubBuildId, GithubClaims, GithubEnd, + GithubGrant, GithubHandOff, GithubReport, GithubRun, SetBuildOrder, }; use serde_json::{Value, json}; use std::collections::BTreeMap; @@ -740,8 +740,6 @@ fn the_build_order_and_preferred_builder_apply_at_once_and_shape_each_walk() { let source = &store.sources(&deployment).unwrap()[0]; assert_eq!(source.builders, [Builder::Github, Builder::Servers]); assert_eq!(source.preferred_machine, None); - assert_eq!(source.sealed, None); - assert!(json!(source).get("sealed").is_none(), "{source:?}"); // A Preferred Server goes first; the walk is read as the build starts. let machine = "0123456789abcdef0123456789abcdef"; @@ -821,6 +819,13 @@ fn lines(from: u64, lines: &[&str], platforms: Option<&[&str]>) -> GithubReport } } +fn held(store: &ConfigStore, id: &GithubBuildId) -> GithubBuild { + match store.github_build(id).unwrap() { + GithubHandOff::Held(build) => *build, + GithubHandOff::PassedOn { why } => panic!("GitHub passed it on: {why}"), + } +} + /// A pinned Git build of a fresh Deployment, handed to GitHub run `RUN`. fn dispatched(store: &ConfigStore, who: &Actor) -> GithubBuildId { store @@ -897,10 +902,10 @@ fn a_github_check_in_must_come_from_the_dispatched_repository_workflow_and_run() ); // Only the runner gets the inputs: the lowering input and the pinned commit. - let (input, commit, receipt) = store.github_input(&id).unwrap(); - assert_eq!(commit.as_str(), "a".repeat(40)); - assert!(input.get("snapshots").is_some()); - assert_eq!(receipt, None); + let built = store.github_input(&id).unwrap(); + assert_eq!(built.commit.as_str(), "a".repeat(40)); + assert!(built.input.get("snapshots").is_some()); + assert_eq!(built.receipt, None); } #[test] @@ -936,7 +941,7 @@ fn a_github_build_checks_in_once_takes_each_log_line_once_and_refuses_late_repor RpcErrorCode::Conflict, "a second check-in is refused" ); - let build = store.github_build(&id).unwrap(); + let build = held(&store, &id); assert_eq!(build.grant, Some(grant())); assert!(build.checked_in_at.is_some()); @@ -969,7 +974,7 @@ fn a_github_build_checks_in_once_takes_each_log_line_once_and_refuses_late_repor .unwrap_err(); assert_eq!(error.code, RpcErrorCode::Conflict); assert_eq!( - store.github_build(&id).unwrap().ended, + held(&store, &id).ended, Some(ployz_store::RunEnd::Built { platforms: vec!["linux/amd64".to_owned()] }) @@ -1064,6 +1069,151 @@ fn a_skipped_github_build_goes_back_to_the_next_builder_and_cancellation_lists_o ); } +#[test] +fn a_build_github_passed_on_at_check_in_reads_as_no_longer_on_github() { + let (store, who) = shop(); + let id = dispatched(&store, &who); + let skip = GithubEnd::Unstarted { + message: "GitHub builds a sealed value only with an up-to-date Ployz".into(), + }; + assert_eq!( + store.github_end(&id, Some(RUN), &skip).unwrap(), + BuildStatus::Pending + ); + assert_eq!( + store.github_build(&id).unwrap(), + GithubHandOff::PassedOn { + why: "GitHub builds a sealed value only with an up-to-date Ployz".into() + } + ); + assert_eq!( + store.github_end(&id, Some(RUN), &skip).unwrap_err().code, + RpcErrorCode::Conflict, + "the walk's later look finds it already ended" + ); + let source = &store.sources(&id.deployment).unwrap()[0]; + assert_eq!( + (source.status, source.message.as_deref()), + ( + Some(BuildStatus::Pending), + Some("GitHub builds a sealed value only with an up-to-date Ployz") + ), + "the next Builder takes it, with why" + ); +} + +#[test] +fn a_run_github_passed_on_holds_nothing_once_the_next_builder_starts() { + for checked_in in [false, true] { + let (store, who) = shop(); + let id = dispatched(&store, &who); + let skip = if checked_in { + store.github_check_in(&id, RUN, &grant()).unwrap(); + GithubEnd::Skipped { + message: "the run ran out of time".into(), + } + } else { + GithubEnd::Unstarted { + message: "GitHub builds a sealed value only with an up-to-date Ployz".into(), + } + }; + store.github_end(&id, Some(RUN), &skip).unwrap(); + + let runner = RunnerId::parse("cloud-1").unwrap(); + store.claim(&id.deployment, &runner).unwrap(); + store + .record( + &id.deployment, + &runner, + report(BuildStatus::Building, None, ""), + ) + .unwrap(); + + let refused = |result: Result<(), ployz_core::RpcError>, what: &str| { + assert_eq!( + result.unwrap_err().code, + RpcErrorCode::Conflict, + "{what} (checked in: {checked_in})" + ); + }; + refused(store.github_check_in(&id, RUN, &grant()), "a late check-in"); + refused( + store + .github_report(&id, RUN, &lines(0, &["late\n"], None)) + .map(drop), + "a late report", + ); + refused( + store + .github_end( + &id, + Some(RUN), + &GithubEnd::Built { + receipt: json!({ "fingerprint": "f".repeat(64) }), + }, + ) + .map(drop), + "a late Built", + ); + refused( + store + .github_end( + &id, + Some(RUN), + &GithubEnd::Unstarted { + message: "no runner started the build in time".into(), + }, + ) + .map(drop), + "a late Unstarted", + ); + refused( + store + .github_end( + &id, + Some(RUN), + &GithubEnd::Failed { + message: "cancelled".into(), + }, + ) + .map(drop), + "a late cancel", + ); + assert!(store.github_outstanding(&id.deployment).unwrap().is_empty()); + assert!( + matches!( + store.github_build(&id).unwrap(), + GithubHandOff::PassedOn { .. } + ), + "GitHub no longer reads as building it" + ); + let source = &store.sources(&id.deployment).unwrap()[0]; + assert_eq!( + source.status, + Some(BuildStatus::Building), + "the Server builds it" + ); + } +} + +#[test] +fn a_late_old_mint_skip_leaves_a_run_that_checked_in_building() { + let (store, who) = shop(); + let id = dispatched(&store, &who); + store.github_check_in(&id, RUN, &grant()).unwrap(); + let late = GithubEnd::Unstarted { + message: "GitHub builds a sealed value only with an up-to-date Ployz".into(), + }; + assert_eq!( + store.github_end(&id, Some(RUN), &late).unwrap_err().code, + RpcErrorCode::Conflict + ); + let build = store.github_authorize(&id, &claims()).unwrap(); + assert_eq!(build.status, BuildStatus::Building); + assert_eq!(build.grant, Some(grant())); + assert_eq!(store.github_outstanding(&id.deployment).unwrap().len(), 1); +} + #[test] fn a_build_clock_starts_at_building_clears_when_skipped_and_reopens_for_a_later_builder() { let (store, who) = shop(); @@ -1191,15 +1341,72 @@ fn a_build_names_the_server_that_built_it_and_why_github_passed_it_on() { } #[test] -fn a_build_github_takes_after_skipping_it_no_longer_says_github_skipped_it() { - let (store, who) = shop(); - let id = dispatched(&store, &who); - let skip = GithubEnd::Skipped { - message: "the run ran out of time".into(), - }; - store.github_end(&id, Some(RUN), &skip).unwrap(); - let skipped = || { - store +fn a_build_github_passed_on_stays_passed_on_for_its_deployment() { + for (checked_in, why) in [ + ( + false, + "GitHub builds a sealed value only with an up-to-date Ployz", + ), + (false, "no runner started the build in time"), + (true, "the run ran out of time"), + ] { + let (store, who) = shop(); + let id = dispatched(&store, &who); + let message = why.to_owned(); + let skip = if checked_in { + store.github_check_in(&id, RUN, &grant()).unwrap(); + GithubEnd::Skipped { message } + } else { + GithubEnd::Unstarted { message } + }; + store.github_end(&id, Some(RUN), &skip).unwrap(); + + let refused = |result: Result<(), ployz_core::RpcError>, what: &str| { + assert_eq!( + result.unwrap_err().code, + RpcErrorCode::Conflict, + "{what} ({why})" + ); + }; + refused( + store.github_dispatched(&id, &github_run(RUN + 1)), + "a retried start's new run", + ); + refused( + store.github_dispatched(&id, &github_run(RUN)), + "the run it passed on", + ); + refused( + store + .github_end( + &id, + None, + &GithubEnd::Skipped { + message: "GitHub couldn't take the build: rate limited".into(), + }, + ) + .map(drop), + "a retried start's skip", + ); + refused( + store + .github_end( + &id, + None, + &GithubEnd::Built { + receipt: json!({ "fingerprint": "f".repeat(64) }), + }, + ) + .map(drop), + "a retried start's reuse", + ); + + assert!(store.github_outstanding(&id.deployment).unwrap().is_empty()); + assert_eq!( + store.github_build(&id).unwrap(), + GithubHandOff::PassedOn { why: why.into() } + ); + let log = store .read( &who, &BuildLogQuery { @@ -1207,23 +1414,14 @@ fn a_build_github_takes_after_skipping_it_no_longer_says_github_skipped_it() { service: ServiceName::parse("web").unwrap(), }, ) - .unwrap() - .build - .github_skipped - }; - assert_eq!(skipped().as_deref(), Some("the run ran out of time")); - - store.github_dispatched(&id, &github_run(RUN + 1)).unwrap(); - assert_eq!(skipped(), None, "a new GitHub run takes the build back"); - store.github_check_in(&id, RUN + 1, &grant()).unwrap(); - let receipt = json!({ "fingerprint": "f".repeat(64), "machine_id": grant().machine }); - assert_eq!( - store - .github_end(&id, Some(RUN + 1), &GithubEnd::Built { receipt }) - .unwrap(), - BuildStatus::Built - ); - assert_eq!(skipped(), None, "GitHub built it"); + .unwrap(); + assert_eq!( + (log.build.status, log.build.github_skipped.as_deref()), + (BuildStatus::Pending, Some(why)), + "the next Builder takes it, with why" + ); + assert_eq!(log.log.matches("GitHub skipped").count(), 1, "{}", log.log); + } } fn source(store: &ConfigStore, who: &Actor) -> SourceKind { @@ -1667,7 +1865,7 @@ fn sealed(service: &str, key: &str) -> Value { } #[test] -fn a_build_that_reads_a_sealed_value_skips_github_whatever_it_prefers() { +fn a_build_that_reads_a_sealed_value_walks_its_builders_like_any_other() { let dir = tempfile::tempdir().unwrap(); let (store, who) = sealed_shop(&dir); store @@ -1682,16 +1880,13 @@ fn a_build_that_reads_a_sealed_value_skips_github_whatever_it_prefers() { .unwrap(); let deployment = admit(&store, &who, 1, &[]); let source = &store.sources(&deployment).unwrap()[0]; - assert_eq!(source.builders, [Builder::Servers]); + assert_eq!(source.builders, [Builder::Github, Builder::Servers]); assert_eq!(source.preferred_machine, None); - assert_eq!(source.sealed.as_deref(), Some("NPM_TOKEN")); - assert_eq!(json!(source)["sealed"], "NPM_TOKEN"); build_order(&store, &who, Some(BuildOrder::GithubOnly)); let source = &store.sources(&deployment).unwrap()[0]; - assert!(source.builders.is_empty(), "{source:?}"); + assert_eq!(source.builders, [Builder::Github]); - // A preferred Server still leads. let machine = "0123456789abcdef0123456789abcdef"; store .write( @@ -1700,7 +1895,7 @@ fn a_build_that_reads_a_sealed_value_skips_github_whatever_it_prefers() { ) .unwrap(); let source = &store.sources(&deployment).unwrap()[0]; - assert_eq!(source.builders, [Builder::Servers]); + assert_eq!(source.builders, [Builder::Servers, Builder::Github]); assert_eq!( source.preferred_machine.as_ref().unwrap().to_string(), machine @@ -1708,7 +1903,7 @@ fn a_build_that_reads_a_sealed_value_skips_github_whatever_it_prefers() { } #[test] -fn github_gets_no_sealed_value_and_never_a_build_that_reads_one() { +fn github_gets_only_its_own_sealed_values() { let dir = tempfile::tempdir().unwrap(); let (store, who) = sealed_shop(&dir); let db = "00000000-0000-4000-8000-000000000004"; @@ -1742,7 +1937,7 @@ fn github_gets_no_sealed_value_and_never_a_build_that_reads_one() { .unwrap(); GithubBuildId::parse(&format!("{deployment}.web")).unwrap() }; - let (input, _, _) = store.github_input(&id).unwrap(); + let input = store.github_input(&id).unwrap().input; let snapshots = input["snapshots"].as_array().unwrap(); assert_eq!(snapshots.len(), 2); for snapshot in snapshots { @@ -1786,34 +1981,13 @@ fn github_gets_no_sealed_value_and_never_a_build_that_reads_one() { let sources = store .pin(&deployment, &pins(&backend::sha(&"b".repeat(40)))) .unwrap(); - assert_eq!(sources[0].sealed.as_deref(), Some("DB_URL")); - assert_eq!(sources[0].builders, [Builder::Servers]); + assert_eq!(sources[0].builders, [Builder::Github, Builder::Servers]); let id = GithubBuildId::parse(&format!("{deployment}.web")).unwrap(); - let error = store.github_input(&id).unwrap_err(); - assert_eq!(error.code, RpcErrorCode::Conflict, "{error:?}"); - assert_eq!( - error.message, - "DB_URL is sealed. Builds that read a sealed value run on your server." - ); -} - -#[test] -fn a_revision_claim_will_refuse_still_lists_its_builds() { - let dir = tempfile::tempdir().unwrap(); - let (store, who) = sealed_shop(&dir); - // PORT reads PASSWORD while it is plain text, and still once it is sealed. - for change in [ - set("web.preferredBuilder", json!("github")), - set("web.env.PASSWORD", json!("8080")), - set("web.env.PORT", json!("${{ PASSWORD }}")), - set("web.env.PASSWORD", sealed(SERVICE, "PASSWORD")), - ] { - store.write(&who, &edit(vec![change])).unwrap(); + let built = store.github_input(&id).unwrap(); + assert!(of(&built.input, SERVICE)["resolvedEnv"]["DB_URL"].is_object()); + for (name, value) in of(&built.input, db)["resolvedEnv"].as_object().unwrap() { + assert!(value.is_string(), "{name} is {value}"); } - let deployment = admit(&store, &who, 1, &[]); - let source = &store.sources(&deployment).unwrap()[0]; - assert_eq!(source.sealed.as_deref(), Some("PASSWORD")); - assert_eq!(source.builders, [Builder::Servers]); } #[test] @@ -1839,13 +2013,10 @@ fn an_environment_with_no_sealed_value_routes_without_opening_a_secret() { let store = ConfigStore::open(&url, SealingKey::new(b"another-key").unwrap()).unwrap(); let id = GithubBuildId::parse(&format!("{deployment}.web")).unwrap(); assert!(store.github_input(&id).is_err()); - // Routing it never did, so it still doesn't. let source = &store.sources(&deployment).unwrap()[0]; - assert_eq!(source.sealed, None); assert_eq!(source.builders, [Builder::Github, Builder::Servers]); let pinned = store .pin(&deployment, &pins(&backend::sha(&"a".repeat(40)))) .unwrap(); assert_eq!(pinned[0].commit, Some(backend::sha(&"a".repeat(40)))); - assert_eq!(pinned[0].sealed, None); } diff --git a/core/crates/ployz-store/tests/sealed_variables.rs b/core/crates/ployz-store/tests/sealed_variables.rs index 8970eada3a..8428b25a4d 100644 --- a/core/crates/ployz-store/tests/sealed_variables.rs +++ b/core/crates/ployz-store/tests/sealed_variables.rs @@ -240,24 +240,6 @@ fn a_sealed_value_is_taken_only_with_the_flag_on_and_only_sealed_for_its_variabl assert!(!format!("{diff:?}").contains("AAAA"), "{diff:?}"); } -#[test] -fn a_read_marks_the_rows_the_browser_sealed_and_only_those() { - let dir = tempfile::tempdir().unwrap(); - let url = backend::fresh_url(&dir); - let store = ConfigStore::open(&url, backend::key()).unwrap(); - shop(&store); - set(&store, "web.env.TOKEN", json!({ "secret": SECRET })); - let read = || store.read(&who(), &EnvironmentQuery::default()).unwrap(); - assert!(json!(read()).get("sealed").is_none()); - backend::sealed_variables_on(&url, "org"); - set(&store, "web.env.PASSWORD", password(1)); - let sealed = read().sealed; - assert_eq!( - sealed.iter().map(ToString::to_string).collect::>(), - [format!("{}:variables.PASSWORD", uuid(3))] - ); -} - #[test] fn a_sealed_secret_is_kept_by_its_marker_and_never_becomes_plain() { let dir = tempfile::tempdir().unwrap(); diff --git a/core/crates/ployz/src/build/inputs.rs b/core/crates/ployz/src/build/inputs.rs index 4551fce712..750801a1ed 100644 --- a/core/crates/ployz/src/build/inputs.rs +++ b/core/crates/ployz/src/build/inputs.rs @@ -104,11 +104,10 @@ impl BuildInputs { pub(super) fn railpack_context( &mut self, path: &Path, - variables: &BTreeMap, + config: Option<&str>, ) -> Result { - let config = variables - .get("RAILPACK_CONFIG_FILE") - .map(|value| value.trim()) + let config = config + .map(str::trim) .filter(|value| !value.is_empty()) .map(PathBuf::from); self.capture_input(Input::Context { @@ -212,13 +211,13 @@ impl BuildInputs { /// /// # Errors /// Fails if the private file cannot be written. - pub(super) fn sealed(&self, capture: &super::sealed::SealedCapture<'_>) -> Result<(), Error> { + pub(super) fn sealed(&self, record: &ployz_core::SealedRecord) -> Result<(), Error> { let bytes = - serde_json::to_vec(capture).map_err(|error| input_error(io::Error::other(error)))?; + serde_json::to_vec(record).map_err(|error| input_error(io::Error::other(error)))?; self.private(&self.root.join("private/sealed.json"), &bytes) } - /// Keep only the Sealed ARG names for the Server Builder. + /// Keep only the Sealed ARG names, for the Builder that opens them. /// /// # Errors /// Fails if the private file cannot be written. diff --git a/core/crates/ployz/src/build/local.rs b/core/crates/ployz/src/build/local.rs index 1242a46aab..1b79180c61 100644 --- a/core/crates/ployz/src/build/local.rs +++ b/core/crates/ployz/src/build/local.rs @@ -2,9 +2,9 @@ use std::{collections::BTreeMap, path::Path}; -use ployz_build::{BuildError, BuiltImage, Output, remote::Definition}; +use ployz_build::{BuildError, BuiltImage, Output, OutputScrub, Staged, remote::Definition}; -use super::{CapturedBuild, CapturedTarget, Error, SEALED_BUILDS_RUN_ON_SERVER, invalid}; +use super::{CapturedBuild, CapturedTarget, Error, invalid}; /// Variables through which a GitHub Actions runner exposes its build cache service. const ACTIONS_CACHE_VARIABLES: [&str; 4] = [ @@ -22,19 +22,27 @@ pub struct LocalImage { impl CapturedBuild { /// Build the single captured target with local Docker and Buildx, rendering progress - /// to stderr and to `observe`. The runner's cache service variables pass through so - /// Buildx can read it. + /// to stderr and to `observe`, both scrubbed of the Sealed values it holds. The + /// runner's cache service variables pass through so Buildx can read it. /// /// # Errors - /// Refuses a capture that is not exactly one target or that reads a Sealed value, - /// and reports the Build's failure. + /// Refuses a capture that is not exactly one target or that reads a Sealed value it + /// doesn't hold, and reports the Build's failure. pub fn execute_local( &self, cancellation: &ployz_build::Cancellation, observe: &(dyn Fn(&ployz_build::Progress) + Sync), ) -> Result { - let (captured, images) = - self.run_local(|request| ployz_build::execute(request, cancellation, observe))?; + let (captured, images) = self.run_local(|request, scrub| { + if let Some(scrub) = scrub + && std::env::var_os("GITHUB_ACTIONS").is_some_and(|value| value == "true") + { + scrub + .mask_github(&mut std::io::stderr().lock()) + .map_err(|error| BuildError::Request(error.to_string()))?; + } + ployz_build::execute(request, cancellation, scrub, observe) + })?; let image = images .into_iter() .next() @@ -50,7 +58,8 @@ impl CapturedBuild { } /// Upload this Build's cache to a GitHub Actions runner's cache service by running the - /// same Build again, producing no image. Without that service it does nothing. + /// same Build again, producing no image. Without that service, or when the Build + /// reads a Sealed value's plaintext, it does nothing: other workflows read that cache. /// /// # Errors /// Refuses a capture that is not exactly one target, and reports the export's failure. @@ -58,33 +67,26 @@ impl CapturedBuild { &self, cancellation: &ployz_build::Cancellation, ) -> Result<(), Error> { - self.run_local(|request| { - ployz_build::export_cache(request, cancellation).map(|()| Vec::new()) + self.run_local(|request, scrub| { + ployz_build::export_cache(request, cancellation, scrub).map(|()| Vec::new()) }) .map(drop) } - /// Run `execute` on the single captured target, with the runner's cache service variables. + /// Run `execute` on the single captured target, with the runner's cache service + /// variables and the Sealed values it holds staged for this solve alone. fn run_local( &self, - execute: impl FnOnce(&ployz_build::Request<'_>) -> Result, BuildError>, + execute: impl FnOnce( + &ployz_build::Request<'_>, + Option<&OutputScrub>, + ) -> Result, BuildError>, ) -> Result<(&CapturedTarget, Vec), Error> { let [captured] = self.targets.as_slice() else { return Err(invalid( "a local Build takes exactly one Git-sourced Service", )); }; - if !captured.sealed.is_empty() { - let names = captured.sealed.join(", "); - let verb = if captured.sealed.len() == 1 { - "is" - } else { - "are" - }; - return Err(Error::Sealed(format!( - "{names} {verb} sealed. {SEALED_BUILDS_RUN_ON_SERVER}" - ))); - } let root = captured.inputs.root(); let definition = Definition { retained_tags: Vec::new(), @@ -94,6 +96,17 @@ impl CapturedBuild { no_cache: false, pull: false, }; + let build = |source| Error::Build { + service: captured.name.clone(), + source, + }; + let staged = self + .opened + .as_ref() + .map(|opened| Staged::stage(root, opened, &definition.targets)) + .transpose() + .map_err(build)?; + let sealed_override = staged.as_ref().and_then(Staged::compose_override); let railpack = ployz_build::remote::validate_capture( root, &definition, @@ -116,10 +129,15 @@ impl CapturedBuild { environment.insert(name.to_owned(), value); } } - let images = execute(&ployz_build::Request { + if ployz_build::reads_plaintext(sealed_override, &railpack) { + // Buildx provenance metadata records build argument values. + environment.insert("BUILDX_METADATA_PROVENANCE".into(), "disabled".into()); + } + let scrub = self.opened.as_ref().map(OutputScrub::new); + let request = ployz_build::Request { image_contexts: &definition.image_contexts, compose_file: Path::new("compose.yaml"), - sealed_override: None, + sealed_override, working_dir: root, environment: &environment, docker: None, @@ -129,12 +147,9 @@ impl CapturedBuild { output: definition.output, no_cache: false, pull: false, - sealed: false, - }) - .map_err(|source| Error::Build { - service: captured.name.clone(), - source, - })?; + sealed: self.opened.is_some(), + }; + let images = execute(&request, scrub.as_ref()).map_err(build)?; Ok((captured, images)) } } @@ -147,18 +162,133 @@ fn path(path: &Path) -> String { mod tests { use super::*; use crate::build::{BuildSpec, Recipe, capture, tests::intent}; + use ployz_core::{DeployIntent, OpenedSealed}; use std::fs; - #[test] - fn a_build_reading_sealed_values_is_refused_here_by_name() { - let root = tempfile::tempdir().unwrap(); - let source = root.path().join("repository"); + const MARKER: &str = "marker-sealed-0123456789"; + + fn dockerfile(root: &Path, dockerfile: &str) -> BuildSpec { + let source = root.join("repository"); fs::create_dir_all(&source).unwrap(); - fs::write(source.join("Dockerfile"), "FROM scratch\n").unwrap(); - let spec = BuildSpec { + fs::write(source.join("Dockerfile"), dockerfile).unwrap(); + BuildSpec { context: source.clone(), recipe: Recipe::Dockerfile(source.join("Dockerfile")), + } + } + + fn sealed(build: &CapturedBuild) -> Option { + let root = build.targets.first().unwrap().inputs.root(); + fs::read_to_string(root.join(ployz_build::sealed_file("NPM_TOKEN"))).ok() + } + + #[test] + fn a_runner_holding_its_sealed_values_stages_them_for_each_solve_alone() { + let root = tempfile::tempdir().unwrap(); + let spec = dockerfile( + root.path(), + "FROM scratch\nARG NPM_TOKEN\nRUN --mount=type=secret,id=DATABASE_URL true\n", + ); + let intent = sealed_intent(); + let name = intent.target.first().unwrap().name.clone(); + let opened = OpenedSealed::new( + BTreeMap::from([ + ("NPM_TOKEN".into(), MARKER.into()), + ("DATABASE_URL".into(), "postgres://marker".into()), + ]), + Vec::new(), + ); + let build = capture(&intent, BTreeMap::from([(name, spec)]), Some(opened)).unwrap(); + for _ in 0..2 { + build + .run_local(|request, scrub| { + assert!(request.sealed && scrub.is_some()); + let args = fs::read_to_string( + request.working_dir.join(request.sealed_override.unwrap()), + ) + .unwrap(); + assert!(args.contains(MARKER), "{args}"); + assert_eq!( + request.environment["BUILDX_METADATA_PROVENANCE"], + "disabled" + ); + assert_eq!(sealed(&build).as_deref(), Some(MARKER)); + Ok(Vec::new()) + }) + .unwrap(); + assert_eq!(sealed(&build), None); + } + } + + #[test] + fn a_build_without_sealed_values_solves_as_it_always_did() { + let root = tempfile::tempdir().unwrap(); + let mut intent = intent(&["api"]); + let name = intent.target.first().unwrap().name.clone(); + intent + .target + .first_mut() + .unwrap() + .container + .environment + .insert("PLAIN".into(), "visible".into()); + let spec = dockerfile(root.path(), "FROM scratch\n"); + let build = capture(&intent, BTreeMap::from([(name, spec)]), None).unwrap(); + build + .run_local(|request, scrub| { + assert!(!request.sealed && scrub.is_none()); + assert_eq!(request.sealed_override, None); + assert!( + !request + .environment + .contains_key("BUILDX_METADATA_PROVENANCE") + ); + Ok(Vec::new()) + }) + .unwrap(); + } + + #[test] + fn a_railpack_build_planned_with_a_sealed_setting_records_no_provenance() { + let root = tempfile::tempdir().unwrap(); + let mut intent = intent(&["api"]); + let container = &mut intent.target.first_mut().unwrap().container; + container.environment.insert( + "RAILPACK_BUILD_CMD".into(), + ployz_core::EnvValue::Sealed(ployz_core::SealedValue { + sealed: ployz_core::Ciphertext::from_bytes(vec![7; 48]), + scope: ployz_core::SealedScope::Shared, + key: "RAILPACK_BUILD_CMD".into(), + }), + ); + container.sealed_identity = Some(ployz_core::SealedIdentity { + organization: "org_test".into(), + environment: uuid::Uuid::from_u128(1), + service: uuid::Uuid::from_u128(2), + }); + let name = intent.target.first().unwrap().name.clone(); + let spec = BuildSpec { + context: root.path().to_owned(), + recipe: Recipe::Railpack { command: None }, }; + let opened = OpenedSealed::new( + BTreeMap::from([("RAILPACK_BUILD_CMD".into(), MARKER.into())]), + Vec::new(), + ); + let build = capture(&intent, BTreeMap::from([(name, spec)]), Some(opened)).unwrap(); + build + .run_local(|request, _| { + assert_eq!(request.sealed_override, None); + assert_eq!( + request.environment["BUILDX_METADATA_PROVENANCE"], + "disabled" + ); + Ok(Vec::new()) + }) + .unwrap(); + } + + fn sealed_intent() -> DeployIntent { let mut intent = intent(&["api"]); let container = &mut intent.target.first_mut().unwrap().container; for name in ["NPM_TOKEN", "DATABASE_URL"] { @@ -176,16 +306,6 @@ mod tests { environment: uuid::Uuid::from_u128(1), service: uuid::Uuid::from_u128(2), }); - let name = intent.target.first().unwrap().name.clone(); - let build = capture(&intent, BTreeMap::from([(name, spec)])).unwrap(); - let error = build - .run_local(|_| panic!("a sealed Build must not reach Docker")) - .err() - .unwrap() - .to_string(); - assert_eq!( - error, - "DATABASE_URL, NPM_TOKEN are sealed. Builds that read a sealed value run on your server." - ); + intent } } diff --git a/core/crates/ployz/src/build/mod.rs b/core/crates/ployz/src/build/mod.rs index 5b85ddd8f6..aa1bc573a6 100644 --- a/core/crates/ployz/src/build/mod.rs +++ b/core/crates/ployz/src/build/mod.rs @@ -11,7 +11,8 @@ use ployz_build::{ remote::{Definition, Outcome}, }; use ployz_core::{ - DeployIntent, MachineId, Placement, RequestedServiceSpec, ServiceName, config::BuildMethod, + DeployIntent, MachineId, OpenedSealed, Placement, RequestedServiceSpec, ServiceName, + config::BuildMethod, }; use serde::Serialize; use thiserror::Error; @@ -30,8 +31,6 @@ mod reuse; pub(crate) use reuse::{holder, image_stores, runs_everywhere}; mod sealed; -pub(crate) use ployz_store::SEALED_BUILDS_RUN_ON_SERVER; - /// How one Service's checked-out source becomes an image. #[derive(Clone, Debug, Eq, PartialEq)] pub struct BuildSpec { @@ -78,7 +77,6 @@ pub enum Error { RemoteBuild { outcome: Box }, #[error("invalid Build: {0}")] Invalid(String), - /// The Build reads Sealed values, which only a Server Builder opens. #[error("{0}")] Sealed(String), #[error("Could not build {service}.")] @@ -123,6 +121,8 @@ impl PartialEq for BuiltService { /// Builds whose sources and settings have already been captured. pub struct CapturedBuild { targets: Vec, + /// The Sealed values a runner fetched for this Build, held for every solve. + opened: Option, } struct CapturedTarget { @@ -141,7 +141,8 @@ struct CapturedTarget { /// Text variables become build variables; a Railpack command is added as /// `RAILPACK_BUILD_CMD`. Build variables may be exposed in build output or history. /// Sealed values stay sealed in private material and reach the Build only as -/// secret mounts the Builder opens. +/// secret mounts the Builder opens. A runner that already holds them `opened` plans +/// a Railpack Build with its sealed settings. /// /// # Errors /// Rejects Services absent from `intent`, unreadable or unstable source files, and @@ -149,6 +150,7 @@ struct CapturedTarget { pub fn capture( intent: &DeployIntent, specs: BTreeMap, + opened: Option, ) -> Result { let targets = specs .into_iter() @@ -158,15 +160,16 @@ pub fn capture( .iter() .find(|service| service.name == name) .ok_or_else(|| invalid(format!("undefined service '{name}'")))?; - capture_target(service, spec) + capture_target(service, spec, opened.as_ref()) }) .collect::>()?; - Ok(CapturedBuild { targets }) + Ok(CapturedBuild { targets, opened }) } fn capture_target( service: &RequestedServiceSpec, spec: BuildSpec, + opened: Option<&OpenedSealed>, ) -> Result { let mut inputs = BuildInputs::new()?; let image = service.container.image.clone(); @@ -180,7 +183,7 @@ fn capture_target( reference.repository(), uuid::Uuid::new_v4() ); - let sealed = sealed::capture(service, &spec.recipe, &inputs)?; + let sealed = sealed::capture(service, &spec.recipe, &inputs, opened)?; let mut args: BTreeMap = service .container .environment @@ -194,7 +197,11 @@ fn capture_target( if let Some(command) = command { args.insert("RAILPACK_BUILD_CMD".into(), command); } - let context = inputs.railpack_context(&spec.context, &args)?; + let config = args + .get("RAILPACK_CONFIG_FILE") + .map(String::as_str) + .or_else(|| opened.and_then(|opened| opened.get("RAILPACK_CONFIG_FILE"))); + let context = inputs.railpack_context(&spec.context, config)?; let context = inputs.relative(&context); // Railpack variables stay in private upload material, not the recipe. inputs.railpack(&ployz_build::Railpack { @@ -453,12 +460,12 @@ fn invalid(message: impl Into) -> Error { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use std::{fs, os::unix::fs::symlink}; /// Image-sourced Cloud Services; each test sets the fields it exercises. - pub(super) fn intent(names: &[&str]) -> DeployIntent { + pub(crate) fn intent(names: &[&str]) -> DeployIntent { let snapshots = names .iter() .map(|name| { @@ -482,7 +489,7 @@ mod tests { spec: BuildSpec, ) -> Result { let name = intent.target.first().unwrap().name.clone(); - let mut captured = capture(intent, BTreeMap::from([(name, spec)]))?; + let mut captured = capture(intent, BTreeMap::from([(name, spec)]), None)?; Ok(captured.targets.pop().unwrap()) } diff --git a/core/crates/ployz/src/build/platforms.rs b/core/crates/ployz/src/build/platforms.rs index f7b65c1941..074c98cebe 100644 --- a/core/crates/ployz/src/build/platforms.rs +++ b/core/crates/ployz/src/build/platforms.rs @@ -232,7 +232,7 @@ mod tests { ), ] .map(|(name, spec)| (ployz_core::ServiceName::parse(name).unwrap(), spec)); - let mut captured = super::super::capture(&intent, specs.into()).unwrap(); + let mut captured = super::super::capture(&intent, specs.into(), None).unwrap(); let platforms = |captured: &CapturedBuild, name: &str| { captured .targets() diff --git a/core/crates/ployz/src/build/sealed.rs b/core/crates/ployz/src/build/sealed.rs index 35917ff00d..c49d72762c 100644 --- a/core/crates/ployz/src/build/sealed.rs +++ b/core/crates/ployz/src/build/sealed.rs @@ -1,12 +1,12 @@ //! Sealed values a Build reads: kept sealed in private material, shaped for its -//! recipe, and opened only by the Server Builder. +//! recipe, and opened only by the Builder. use std::{ collections::BTreeMap, path::{Path, PathBuf}, }; -use ployz_core::{DockerfileFacts, EnvValue, RequestedServiceSpec}; +use ployz_core::{DockerfileFacts, EnvValue, OpenedSealed, RequestedServiceSpec, SealedRecord}; use serde::Serialize; use sha2::{Digest, Sha256}; @@ -28,7 +28,8 @@ pub(super) struct Sealed { /// Keep the Sealed values `service` reads in private upload material and shape /// them for `recipe`. A Dockerfile is checked once captured, by -/// [`declared_args`]. +/// [`declared_args`]. Only a capture with the values `opened` may seal a +/// Railpack setting, which Railpack then plans with. /// /// # Errors /// Fails without the Deploy's Environment to seal for, when the private file @@ -37,38 +38,32 @@ pub(super) fn capture( service: &RequestedServiceSpec, recipe: &Recipe, inputs: &BuildInputs, + opened: Option<&OpenedSealed>, ) -> Result { - let values: BTreeMap = service - .container - .environment - .iter() - .filter(|(_, value)| value.text().is_none()) - .map(|(key, value)| (key.clone(), value.clone())) - .collect(); let mut sealed = Sealed::default(); - if values.is_empty() { + let Some(record) = SealedRecord::of(service).map_err(|error| invalid(error.to_string()))? + else { return Ok(sealed); - } - inputs.sealed(&SealedCapture { - identity: service - .container - .sealed_identity - .as_ref() - .ok_or_else(|| invalid("a sealed value needs the Deploy's Environment"))?, - values: &values, - })?; + }; + inputs.sealed(&record)?; + let values = record.values(); match recipe { - Recipe::Railpack { .. } => { - refuse_railpack_settings(values.keys())?; - sealed.fingerprints = values + Recipe::Railpack { command } => { + // An explicit build command wins, as it does over a plain value. + let planned = values .iter() + .filter(|(name, _)| command.is_none() || name.as_str() != "RAILPACK_BUILD_CMD"); + if opened.is_none() { + refuse_railpack_settings(planned.clone().map(|(name, _)| name))?; + } + sealed.fingerprints = planned .map(|(name, value)| (name.clone(), fingerprint(value))) .collect(); } Recipe::Dockerfile(_) => { sealed .args - .insert(SEALED_HASH_ARG.into(), sealed_hash(&values)); + .insert(SEALED_HASH_ARG.into(), sealed_hash(values)); sealed.secrets = values .keys() .map(|name| { @@ -78,7 +73,7 @@ pub(super) fn capture( .collect(); } } - sealed.names = values.into_keys().collect(); + sealed.names = record.keys().map(str::to_owned).collect(); Ok(sealed) } @@ -142,14 +137,6 @@ fn sealed_hash(sealed: &BTreeMap) -> String { .collect() } -/// `private/sealed.json`: the Sealed values a Build reads, still sealed, in -/// the shape the Builder's env opening already accepts. -#[derive(Serialize)] -pub(super) struct SealedCapture<'value> { - identity: &'value ployz_core::SealedIdentity, - values: &'value BTreeMap, -} - /// One Sealed value as a Compose secret: the file the Builder opens it into. #[derive(Serialize)] pub(super) struct CapturedSecret { @@ -434,7 +421,7 @@ mod tests { unreachable!() }; let mut inputs = BuildInputs::new().unwrap(); - let sealed = capture(service, &spec.recipe, &inputs).unwrap(); + let sealed = capture(service, &spec.recipe, &inputs, None).unwrap(); fs::write(source, captured).unwrap(); let file = inputs.dockerfile(source).unwrap(); fs::write(source, later).unwrap(); @@ -494,6 +481,125 @@ RUN --mount=type=secret,id=RAILPACK_BUILD_CMD true capture_one(&intent, railpack()).unwrap(); } + #[test] + #[expect(clippy::indexing_slicing, reason = "fixed capture fixture")] + fn a_runner_holding_its_sealed_values_plans_railpack_with_them() { + let root = tempfile::tempdir().unwrap(); + let source = root.path().join("repository"); + fs::create_dir_all(source.join("config")).unwrap(); + fs::write( + source.join("config/custom.json"), + r#"{"exclude": ["drop.log"]}"#, + ) + .unwrap(); + fs::write(source.join("drop.log"), "excluded").unwrap(); + let (mut intent, _) = sealed_intent(7); + let setting = EnvValue::Sealed(ployz_core::SealedValue { + sealed: ployz_core::Ciphertext::from_bytes(vec![9; 48]), + scope: ployz_core::SealedScope::Shared, + key: "RAILPACK_CONFIG_FILE".into(), + }); + intent + .target + .first_mut() + .unwrap() + .container + .environment + .insert("RAILPACK_CONFIG_FILE".into(), setting); + let opened = OpenedSealed::new( + BTreeMap::from([ + ("NPM_TOKEN".into(), "marker-sealed-token".into()), + ("RAILPACK_CONFIG_FILE".into(), "config/custom.json".into()), + ]), + Vec::new(), + ); + let name = intent.target.first().unwrap().name.clone(); + let spec = BuildSpec { + context: source, + recipe: Recipe::Railpack { command: None }, + }; + let mut build = + crate::build::capture(&intent, BTreeMap::from([(name, spec)]), Some(opened)).unwrap(); + let captured = build.targets.pop().unwrap(); + let root = captured.inputs.root(); + let compose: serde_norway::Value = + serde_norway::from_slice(&fs::read(root.join("compose.yaml")).unwrap()).unwrap(); + let context = root.join( + compose["services"]["api"]["build"]["context"] + .as_str() + .unwrap(), + ); + assert!(context.join("config/custom.json").exists()); + assert!(!context.join("drop.log").exists()); + let railpack = fs::read_to_string(root.join("private/railpack.json")).unwrap(); + let private: serde_json::Value = serde_json::from_str(&railpack).unwrap(); + assert!( + private[0]["variables"] + .get("RAILPACK_CONFIG_FILE") + .is_none() + ); + assert_eq!( + private[0]["sealed"]["RAILPACK_CONFIG_FILE"] + .as_str() + .unwrap() + .len(), + 64 + ); + assert!(!railpack.contains("marker-sealed-token")); + } + + #[test] + #[expect(clippy::indexing_slicing, reason = "fixed capture fixture")] + fn an_explicit_build_command_wins_over_a_sealed_one() { + let root = tempfile::tempdir().unwrap(); + let (mut intent, _) = sealed_intent(7); + let setting = EnvValue::Sealed(ployz_core::SealedValue { + sealed: ployz_core::Ciphertext::from_bytes(vec![9; 48]), + scope: ployz_core::SealedScope::Shared, + key: "RAILPACK_BUILD_CMD".into(), + }); + intent + .target + .first_mut() + .unwrap() + .container + .environment + .insert("RAILPACK_BUILD_CMD".into(), setting); + let railpack = || BuildSpec { + context: root.path().to_owned(), + recipe: Recipe::Railpack { + command: Some("make release".into()), + }, + }; + let opened = OpenedSealed::new( + BTreeMap::from([ + ("NPM_TOKEN".into(), "marker-sealed-token".into()), + ("RAILPACK_BUILD_CMD".into(), "marker-sealed-command".into()), + ]), + Vec::new(), + ); + let name = intent.target.first().unwrap().name.clone(); + for opened in [Some(opened), None] { + let mut build = crate::build::capture( + &intent, + BTreeMap::from([(name.clone(), railpack())]), + opened, + ) + .unwrap(); + let captured = build.targets.pop().unwrap(); + let railpack = + fs::read_to_string(captured.inputs.root().join("private/railpack.json")).unwrap(); + let private: serde_json::Value = serde_json::from_str(&railpack).unwrap(); + assert_eq!( + private[0]["variables"]["RAILPACK_BUILD_CMD"], + "make release" + ); + assert!(private[0]["sealed"].get("RAILPACK_BUILD_CMD").is_none()); + assert!(private[0]["sealed"].get("NPM_TOKEN").is_some()); + assert!(!railpack.contains("marker-sealed")); + } + } + #[test] fn a_sealed_value_without_an_identity_cannot_be_captured() { let root = tempfile::tempdir().unwrap(); diff --git a/core/crates/ployz/src/connect/build_grant.rs b/core/crates/ployz/src/connect/build_grant.rs index 78da01457a..7f7010593b 100644 --- a/core/crates/ployz/src/connect/build_grant.rs +++ b/core/crates/ployz/src/connect/build_grant.rs @@ -7,7 +7,9 @@ use iroh::{ Endpoint, EndpointAddr, PublicKey, RelayMode, SecretKey, endpoint::{Connection, ConnectionError, VarInt, presets}, }; -use ployz_core::{BUILD_GRANT_ALPN, BUILD_GRANT_ENDED, BUILD_GRANT_REFUSED, BuildGrant}; +use ployz_core::{ + BUILD_GRANT_ALPN, BUILD_GRANT_ENDED, BUILD_GRANT_REFUSED, BuildGrant, OpenedSealed, +}; use tokio::net::TcpListener; use super::ManagementRelay; @@ -42,6 +44,38 @@ impl GrantRegistry { None } } + + /// The Sealed values the Machine opened for this grant, which it serves only to the + /// grant's holder before the push. + /// + /// # Errors + /// Reports the Machine's refusal or an unreadable answer. + pub async fn sealed(&self) -> io::Result { + let response = reqwest::Client::builder() + .no_proxy() + .build() + .map_err(io::Error::other)? + .get(format!("http://{}/ployz/sealed", self.address)) + .send() + .await + .map_err(|error| self.refused(error.to_string()))?; + if response.status().is_success() { + return response.json().await.map_err(io::Error::other); + } + let status = response.status(); + let body: serde_json::Value = response.json().await.unwrap_or_default(); + let message = body + .pointer("/errors/0/message") + .and_then(serde_json::Value::as_str) + .unwrap_or_else(|| status.canonical_reason().unwrap_or("refused")); + Err(io::Error::other(format!( + "the Machine did not open the Build's Sealed values: {message}" + ))) + } + + fn refused(&self, error: String) -> io::Error { + io::Error::other(self.refusal().map_or(error, str::to_owned)) + } } impl Drop for GrantRegistry { @@ -98,3 +132,21 @@ pub async fn open_grant_registry( _endpoint: endpoint, }) } + +#[cfg(test)] +mod tests { + use iroh::SecretKey; + + #[test] + fn a_holder_seed_names_the_key_the_contract_pins() { + let seed: [u8; 32] = + hex::decode("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") + .unwrap() + .try_into() + .unwrap(); + assert_eq!( + hex::encode(SecretKey::from_bytes(&seed).public().as_bytes()), + "d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a" + ); + } +} diff --git a/core/crates/ployz/src/handlers/build.rs b/core/crates/ployz/src/handlers/build.rs index 4c93396b97..9039683bd2 100644 --- a/core/crates/ployz/src/handlers/build.rs +++ b/core/crates/ployz/src/handlers/build.rs @@ -3,23 +3,27 @@ //! Machine with a Build Grant. It knows nothing of the CI system running it; it uses //! the GitHub Actions cache when that runtime's environment is exported to it. -use std::{collections::BTreeMap, path::Path, process::Command}; +use std::{collections::BTreeMap, path::Path, process::Command, sync::Arc}; use clap::{ArgMatches, ValueHint}; +use tokio_util::sync::CancellationToken; use crate::cli::{env, value}; use ployz_core::{ - BuildGrant, RETAINED_DIGEST_TAG_PREFIX, ServiceName, + BuildGrant, OpenedSealed, RETAINED_DIGEST_TAG_PREFIX, SealedRecord, ServiceName, config::{ServiceSource, parse_service_config}, }; use serde_json::{Value, json}; use super::{Error, leaf_matches, required, runtime}; use crate::{ - build::LocalImage, + build::{CapturedBuild, LocalImage}, connect::{ManagementRelay, open_grant_registry}, failure::Failure, - sdk::{PreparationInput, preparation::capture}, + sdk::{ + PreparationInput, expected_fingerprints, + preparation::{capture_opened, frozen_intent}, + }, }; /// Why `ployz build` refused or failed after its arguments were accepted. @@ -44,6 +48,10 @@ enum BuildCommandError { NotDigestIdentified, #[error("{0}")] GrantRefused(&'static str), + #[error("{0}")] + Sealed(String), + #[error("the build was cancelled")] + Cancelled, } impl From for Failure { @@ -70,57 +78,134 @@ pub(super) fn build(root: &ArgMatches) -> Result<(), Error> { return Err(Error::usage("--commit must be a full lowercase Git SHA")); } check_out(&source, &commit)?; - let captured = capture(PreparationInput { - deployment, - sources: BTreeMap::from([(service.clone(), source)]), - source_commits: BTreeMap::from([(service.clone(), commit)]), - ..PreparationInput::default() - })?; - if captured - .fingerprints - .get(&service) - .map(|(fingerprint, _)| fingerprint) - != Some(&expected) - { + let fingerprint = expected_fingerprints( + deployment.clone(), + BTreeMap::from([(service.clone(), commit.clone())]), + )? + .remove(&service); + if fingerprint.as_ref() != Some(&expected) { return Err(BuildCommandError::FingerprintMismatch { version: env!("CARGO_PKG_VERSION"), } .into()); } - let events = std::sync::Arc::new(Events( + let events = Arc::new(Events( matches .get_one::("events") .map(std::fs::File::create) .transpose()? .map(std::sync::Mutex::new), )); - runtime()?.block_on(async { - let cancellation = crate::cancellation::on_ctrl_c(); - let build = captured.build; - let export = cancellation.clone(); - let building = events.clone(); - let (build, built) = tokio::task::spawn_blocking(move || { - let built = build.execute_local(&cancellation, &|event| building.write(event)); - (build, built) - }) + // A CI runner's TERM must unwind the staged values and capture, not kill the process. + let (cancellation, _sigterm) = crate::cancellation::interrupted_or_terminated()?; + let runtime = runtime()?; + let opened = runtime.block_on(until_cancelled( + &cancellation, + sealed(&grant, &deployment, &service, &events), + ))?; + let captured = capture_opened( + PreparationInput { + deployment, + sources: BTreeMap::from([(service.clone(), source)]), + source_commits: BTreeMap::from([(service, commit)]), + ..PreparationInput::default() + }, + opened, + )?; + runtime.block_on(run(captured.build, &grant, events, &cancellation)) +} + +/// Build `build` locally, push it through `grant` and export its cache, stopping at +/// `cancellation`. The Build's staged values and capture are gone when this returns. +async fn run( + build: CapturedBuild, + grant: &BuildGrant, + events: Arc, + cancellation: &CancellationToken, +) -> Result<(), Error> { + let token = cancellation.clone(); + let building = Arc::clone(&events); + let (build, built) = tokio::task::spawn_blocking(move || { + let built = build.execute_local(&token, &|event| building.write(event)); + (build, built) + }) + .await + .map_err(std::io::Error::other)?; + push( + grant, + &built.map_err(BuildCommandError::from)?, + &events, + cancellation, + ) + .await?; + // The pushed image and its printed result are final; uploading cache only speeds up + // the next Build, so its failure is a warning and it never reaches the events file. + let export = cancellation.clone(); + let exported = tokio::task::spawn_blocking(move || build.export_local_cache(&export)) .await .map_err(std::io::Error::other)?; - push(&grant, &built.map_err(BuildCommandError::from)?, &events).await?; - // The pushed image and its printed result are final; uploading cache only speeds up - // the next Build, so its failure is a warning and it never reaches the events file. - let exported = tokio::task::spawn_blocking(move || build.export_local_cache(&export)) - .await - .map_err(std::io::Error::other)?; - if let Err(error) = exported { - crate::ui::warn_cause( - "the image was pushed, but its build cache was not exported", - &error, - ); - } - Ok(()) + if let Err(error) = exported { + crate::ui::warn_cause( + "the image was pushed, but its build cache was not exported", + &error, + ); + } + Ok(()) +} + +async fn until_cancelled( + cancellation: &CancellationToken, + work: impl Future>, +) -> Result { + tokio::select! { + done = work => done, + () = cancellation.cancelled() => Err(BuildCommandError::Cancelled.into()), + } +} + +/// The Sealed values `service` reads, opened by the grant's Machine. A failure reaches +/// the events file only as a fixed line, naming at most a variable. +async fn sealed( + grant: &BuildGrant, + deployment: &Value, + service: &ServiceName, + events: &Events, +) -> Result, Error> { + let intent = frozen_intent(deployment.clone())?; + let record = intent + .target + .iter() + .find(|spec| spec.name == *service) + .map(SealedRecord::of) + .transpose() + .map_err(|error| BuildCommandError::Sealed(error.to_string()))? + .flatten(); + let Some(record) = record else { + return Ok(None); + }; + let fetched = async { + let registry = open_grant_registry(grant, &ManagementRelay::default()).await?; + Ok(registry.sealed().await?) + } + .await; + covering(fetched, &record, events).map(Some) +} + +fn covering( + fetched: Result, + record: &SealedRecord, + events: &Events, +) -> Result { + let opened = fetched.inspect_err(|_| events.push_line(SEALED_UNREAD))?; + opened.covering(record.keys()).map_err(|missing| { + let missing = missing.to_string(); + events.push_line(&missing); + BuildCommandError::Sealed(missing).into() }) } +const SEALED_UNREAD: &str = "the Server did not open this Build's sealed values"; + /// The `--events` file: one `{"at": , "event": {"Build": }}` line per /// event, the SDK preparation event shape Cloud's build log reads. Progress is best effort: /// a failed write never fails the build. @@ -198,17 +283,25 @@ fn git(source: &Path, args: &[&str]) -> Result { /// Push through the grant with `docker push`, which sends only the layers the Machine /// lacks, and print what the Machine verified it received. Each layer's progress line /// goes to the events file as the push's output. -async fn push(grant: &BuildGrant, built: &LocalImage, events: &Events) -> Result<(), Error> { +async fn push( + grant: &BuildGrant, + built: &LocalImage, + events: &Events, + cancellation: &CancellationToken, +) -> Result<(), Error> { let digest = &built.image.reference; let hex = digest .strip_prefix("sha256:") .ok_or(BuildCommandError::NotDigestIdentified)?; let tag = format!("{}:{RETAINED_DIGEST_TAG_PREFIX}{hex}", built.repository); events.write(&ployz_build::Progress::Stage(ployz_build::Stage::Push)); - let registry = open_grant_registry(grant, &ManagementRelay::default()).await?; + let registry = until_cancelled(cancellation, async { + Ok(open_grant_registry(grant, &ManagementRelay::default()).await?) + }) + .await?; let local = format!("{}/{tag}", registry.address()); docker(&["tag", digest, &local]).await?; - let pushed = docker_push(&local, events).await; + let pushed = until_cancelled(cancellation, docker_push(&local, events)).await; // The loopback name is meaningless once this command exits. let _ = docker(&["image", "rm", &local]).await; if let Err(error) = pushed { @@ -236,6 +329,7 @@ async fn docker_push(reference: &str, events: &Events) -> Result<(), Error> { .args(["push", reference]) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) + .kill_on_drop(true) .spawn()?; let stderr = child.stderr.take().map(|stderr| { tokio::spawn(async move { @@ -319,3 +413,220 @@ pub(super) fn command(command: clap::Command) -> clap::Command { .help("Also write build progress to this file, one JSON line per event"), ) } + +#[cfg(test)] +mod tests { + use std::fs; + + use ployz_core::{EnvValue, SealedIdentity, SealedScope, SealedValue}; + + use super::*; + + const MARKER: &str = "marker-sealed-0123456789"; + + fn sealed_record() -> SealedRecord { + let value = EnvValue::Sealed(SealedValue { + sealed: ployz_core::Ciphertext::from_bytes(vec![7; 48]), + scope: SealedScope::Shared, + key: "NPM_TOKEN".into(), + }); + SealedRecord::new( + SealedIdentity { + organization: "org_test".into(), + environment: uuid::Uuid::from_u128(1), + service: uuid::Uuid::from_u128(2), + }, + BTreeMap::from([("NPM_TOKEN".into(), value)]), + ) + } + + fn events() -> (tempfile::TempDir, Events) { + let directory = tempfile::tempdir().unwrap(); + let file = fs::File::create(directory.path().join("events")).unwrap(); + (directory, Events(Some(std::sync::Mutex::new(file)))) + } + + /// The build output the events file carries. + fn written(directory: &tempfile::TempDir) -> String { + fs::read_to_string(directory.path().join("events")) + .unwrap() + .lines() + .map(|line| { + let event: Value = serde_json::from_str(line).unwrap(); + let bytes: Vec = + serde_json::from_value(event.pointer("/event/Build/Output").unwrap().clone()) + .unwrap(); + String::from_utf8(bytes).unwrap() + }) + .collect() + } + + #[test] + fn a_failed_fetch_reaches_the_events_file_as_a_fixed_line() { + let (directory, events) = events(); + let machine = std::io::Error::other("the Machine did not open them: machine detail"); + let error = covering(Err(machine.into()), &sealed_record(), &events).unwrap_err(); + assert!(error.to_string().contains("machine detail"), "{error}"); + let written = written(&directory); + assert!(written.contains(SEALED_UNREAD), "{written}"); + assert!(!written.contains("machine detail"), "{written}"); + } + + #[test] + fn a_fetch_missing_a_value_names_only_the_variable() { + let (directory, events) = events(); + let opened = OpenedSealed::new( + BTreeMap::from([("OTHER".into(), MARKER.into())]), + Vec::new(), + ); + covering(Ok(opened), &sealed_record(), &events).unwrap_err(); + let written = written(&directory); + assert!(written.contains("NPM_TOKEN"), "{written}"); + assert!(!written.contains(MARKER), "{written}"); + } + + #[cfg(target_os = "linux")] + mod terminated { + use std::time::Duration; + + use ployz_core::ManagementIdentity; + + use super::*; + use crate::build::{BuildSpec, Recipe}; + + const CHILD: &str = "PLOYZ_BUILD_TERM_CHILD"; + + /// Every capture under `tmp` and the Sealed files staged in it. + fn staged(tmp: &Path) -> (usize, usize) { + let captures = fs::read_dir(tmp) + .unwrap() + .flatten() + .filter(|entry| { + entry + .file_name() + .to_string_lossy() + .starts_with("ployz-build-") + }) + .collect::>(); + let sealed = captures + .iter() + .filter(|capture| capture.path().join("private/sealed/NPM_TOKEN").exists()) + .count(); + (captures.len(), sealed) + } + + #[test] + fn a_sigterm_mid_build_unwinds_its_staged_values_and_capture() { + use std::os::unix::fs::PermissionsExt as _; + let directory = tempfile::tempdir().unwrap(); + let bin = directory.path().join("bin"); + let tmp = directory.path().join("tmp"); + fs::create_dir_all(&bin).unwrap(); + fs::create_dir_all(&tmp).unwrap(); + // The solve hangs until it is stopped; teardown succeeds, so the + // builder's state is confirmed and not quarantined. + fs::write( + bin.join("docker"), + r#"#!/bin/sh + case "$1 $2" in + 'context show') echo default ;; + 'info --format') echo '{"DriverStatus":[["driver-type","io.containerd.snapshotter.v1"]],"Architecture":"amd64","OSType":"linux","CgroupVersion":"2"}' ;; + 'buildx ls') echo "{\"Name\":\"ployz-$(id -u)\",\"Nodes\":[{\"Status\":\"running\",\"Platforms\":[\"linux/amd64\"]}]}" ;; + 'buildx bake') echo $$ >> "$0.pids"; exec sleep 60 ;; + esac + exit 0 + "#, + ) + .unwrap(); + fs::set_permissions(bin.join("docker"), fs::Permissions::from_mode(0o700)).unwrap(); + let path = format!( + "{}:{}", + bin.display(), + std::env::var("PATH").unwrap_or_default() + ); + let mut child = Command::new(std::env::current_exe().unwrap()) + .args([ + "handlers::build::tests::terminated::a_build_runs_until_its_process_is_terminated", + "--exact", + "--ignored", + "--nocapture", + ]) + .env(CHILD, "1") + .env("PATH", path) + .env("TMPDIR", &tmp) + .spawn() + .unwrap(); + let pids = bin.join("docker.pids"); + let mut waited = Duration::ZERO; + while fs::read_to_string(&pids).map_or(true, |pids| !pids.ends_with('\n')) { + assert!( + waited < Duration::from_secs(30), + "the child never started its build" + ); + // real-wait: the Docker stand-in is another process; its start emits no event to await + std::thread::sleep(Duration::from_millis(20)); + waited += Duration::from_millis(20); + } + assert_eq!(staged(&tmp), (1, 1), "the build staged its Sealed value"); + Command::new("kill") + .args(["-TERM", &child.id().to_string()]) + .status() + .unwrap(); + let mut waited = Duration::ZERO; + let status = loop { + if let Some(status) = child.try_wait().unwrap() { + break status; + } + assert!( + waited < Duration::from_secs(30), + "the build ignored SIGTERM" + ); + // real-wait: the child is another process; its exit emits no event to await + std::thread::sleep(Duration::from_millis(50)); + waited += Duration::from_millis(50); + }; + assert!(status.success(), "{status}"); + assert_eq!(staged(&tmp), (0, 0)); + let pids = fs::read_to_string(&pids).unwrap(); + for pid in pids.lines() { + let alive = Command::new("kill").args(["-0", pid]).status().unwrap(); + assert!(!alive.success(), "docker {pid} outlived the build"); + } + } + + #[test] + #[ignore = "child of a_sigterm_mid_build_unwinds_its_staged_values_and_capture"] + fn a_build_runs_until_its_process_is_terminated() { + assert!(std::env::var_os(CHILD).is_some()); + let (cancellation, _sigterm) = + crate::cancellation::interrupted_or_terminated().unwrap(); + let mut intent = crate::build::tests::intent(&["api"]); + let record = sealed_record(); + let container = &mut intent.target.first_mut().unwrap().container; + container.environment.extend(record.values().clone()); + container.sealed_identity = Some(record.identity().clone()); + let source = tempfile::tempdir().unwrap(); + fs::write( + source.path().join("Dockerfile"), + "FROM scratch\nRUN --mount=type=secret,id=NPM_TOKEN true\n", + ) + .unwrap(); + let spec = BuildSpec { + context: source.path().to_owned(), + recipe: Recipe::Dockerfile(source.path().join("Dockerfile")), + }; + let opened = OpenedSealed::new( + BTreeMap::from([("NPM_TOKEN".into(), MARKER.into())]), + Vec::new(), + ); + let name = intent.target.first().unwrap().name.clone(); + let build = + crate::build::capture(&intent, BTreeMap::from([(name, spec)]), Some(opened)) + .unwrap(); + let grant = BuildGrant::new(ManagementIdentity::from_bytes([7; 32]), [3; 32]); + let runtime = tokio::runtime::Runtime::new().unwrap(); + let built = runtime.block_on(run(build, &grant, Arc::new(Events(None)), &cancellation)); + assert!(built.is_err()); + } + } +} diff --git a/core/crates/ployz/src/sdk.rs b/core/crates/ployz/src/sdk.rs index 301b619aff..72cc688bc1 100644 --- a/core/crates/ployz/src/sdk.rs +++ b/core/crates/ployz/src/sdk.rs @@ -40,8 +40,8 @@ mod store_runner; pub use build::{BuildOutcome, OutsideBuild}; pub use deploy::ImageCleanup; pub use github_build::{ - GithubCheckIn, GithubFinish, GithubReported, GithubStart, github_cancel, github_check_in, - github_finish, github_report, github_start, + GithubCheckIn, GithubFinish, GithubKey, GithubReported, GithubStart, github_cancel, + github_check_in, github_finish, github_report, github_start, }; pub use running::Running; pub use store_call::store_call; @@ -1514,12 +1514,8 @@ pub(crate) fn preparation_error( .flatten() .filter_map(Value::as_str) .collect::>(); - const NO_BUILDER: &str = - "No eligible Build Machine was selected; no build was started."; - let message = match details.get("lead").and_then(Value::as_str) { - Some(lead) => format!("{lead} {NO_BUILDER}"), - None => NO_BUILDER.into(), - }; + let message = + "No eligible Build Machine was selected; no build was started.".to_owned(); RpcError { code: RpcErrorCode::Unavailable, details: serde_json::json!({"preparation":{"kind":"failed", "stage":"Selection", diff --git a/core/crates/ployz/src/sdk/github_build.rs b/core/crates/ployz/src/sdk/github_build.rs index a298e5f01a..fbd7bb8be8 100644 --- a/core/crates/ployz/src/sdk/github_build.rs +++ b/core/crates/ployz/src/sdk/github_build.rs @@ -8,16 +8,17 @@ //! A run that never checked in, or that GitHub failed for its own reasons, is //! skipped: the next Builder of the build's walk takes it. A failed build step fails it. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::sync::Arc; use ployz_core::{ - BuildGrantId, BuildGrantRepository, InspectBuildGrantRequest, MintBuildGrantRequest, - RETAINED_DIGEST_TAG_PREFIX, RpcError, RpcErrorCode, + BUILD_GRANT_HOLDER_CAPABILITY, BuildGrantId, BuildGrantRepository, InspectBuildGrantRequest, + MintBuildGrantRequest, MintedKey, RETAINED_DIGEST_TAG_PREFIX, RpcError, RpcErrorCode, + SealedRecord, ServiceName, }; use ployz_store::{ - BuildStatus, ConfigStore, GithubBuildId, GithubClaims, GithubEnd, GithubGrant, GithubReport, - RunEnd, + BuildStatus, ConfigStore, GithubBuildId, GithubClaims, GithubEnd, GithubGrant, GithubHandOff, + GithubReport, RunEnd, }; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -54,13 +55,16 @@ pub async fn github_start( id: GithubBuildId, connections: Vec, ) -> Result { - let (input, commit, receipt) = store_call(&store, { + let built = store_call(&store, { let id = id.clone(); move |store| store.github_input(&id) }) .await?; - let deployment = only(&input, &id.service); - let receipt = receipt.and_then(|receipt| serde_json::from_value(receipt).ok()); + let deployment = only(&built.input, &id.service); + let commit = built.commit; + let receipt = built + .receipt + .and_then(|receipt| serde_json::from_value(receipt).ok()); let outside = match session(connections).await { Ok(session) => { let outside = outside(&session, deployment, commit.to_string(), receipt).await; @@ -128,13 +132,12 @@ async fn outside( } } -/// What a runner receives at check-in. Holds the grant and the build secrets. +/// What a runner receives at check-in. Holds the build secrets. #[derive(Clone, Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct GithubCheckIn { - /// The Build Grant's secret: it lets the runner push into this Service's one - /// repository, and nothing else. - pub grant: String, + #[serde(flatten)] + pub key: GithubKey, /// The pinned commit the runner checks out and builds. pub commit: String, /// The build's fingerprint; the runner's `ployz build` refuses to build any other. @@ -145,17 +148,36 @@ pub struct GithubCheckIn { pub deployment: Value, } +/// How a checked-in runner pushes into this Service's one repository. +#[derive(Clone, Debug, Serialize)] +#[serde(untagged)] +pub enum GithubKey { + /// The Build Grant's secret, minted by the Machine. + Grant { + /// `ployzgrant1:` and the Machine and secret key, unpadded base64url. + grant: String, + }, + /// The runner's own key holds the grant. + Machine { + /// The Management Identity of the Machine to dial, as lowercase hex. + machine: String, + }, +} + /// A runner's one check-in, with the OIDC `claims` Cloud verified: the build -/// starts. Mints a Build Grant on a Machine and returns it with the Service's build -/// inputs. Accepted once, while GitHub still holds the build. +/// starts. Mints a Build Grant on a Machine, for the runner's `holder` key when it +/// sent one, and returns it with the Service's build inputs. Accepted once, while +/// GitHub still holds the build. /// /// # Errors /// Returns `unauthenticated` for a token of another repository, workflow or run, -/// `conflict` once it checked in or is no longer wanted, or the Machine's refusal. +/// `conflict` once it checked in or is no longer wanted or when only the next +/// Builder can build it, or the Machine's refusal. pub async fn github_check_in( store: Arc, id: GithubBuildId, claims: GithubClaims, + holder: Option, connections: Vec, ) -> Result { let build = store_call(&store, { @@ -168,22 +190,37 @@ pub async fn github_check_in( "This build already checked in or is no longer wanted", )); } - let (input, commit, _) = store_call(&store, { + let run_id = build.run.run_id; + let built = store_call(&store, { let id = id.clone(); move |store| store.github_input(&id) }) .await?; - let deployment = only(&input, &id.service); + let deployment = only(&built.input, &id.service); + let commit = built.commit; let fingerprint = super::expected_fingerprints( deployment.clone(), BTreeMap::from([(id.service.clone(), commit.to_string())]), )? .remove(&id.service) .ok_or_else(|| internal("The build has no fingerprint"))?; + let sealed = sealed_record(&deployment, &id.service)?; let session = session(connections).await?; - let minted = mint(&session, &id).await; + let minted = mint(&session, &id, holder, sealed).await; let (machine, minted) = match minted { - Ok(minted) => minted, + Ok(Some(minted)) => minted, + Ok(None) => { + session.close().await; + let end = GithubEnd::Unstarted { + message: OLD_MINT_SKIPS.to_owned(), + }; + store_call(&store, { + let id = id.clone(); + move |store| store.github_end(&id, Some(run_id), &end) + }) + .await?; + return Err(conflict(OLD_MINT_SKIPS)); + } Err(error) => { session.close().await; return Err(error); @@ -194,7 +231,6 @@ pub async fn github_check_in( machine, fingerprint: fingerprint.clone(), }; - let run_id = build.run.run_id; let checked_in = store_call(&store, { let id = id.clone(); move |store| store.github_check_in(&id, run_id, &grant) @@ -204,8 +240,16 @@ pub async fn github_check_in( // Lost to another check-in or the walk moving on: the secret never left, and the // grant expires by itself. checked_in?; + let key = match minted.key { + MintedKey::Grant { grant } => GithubKey::Grant { + grant: grant.to_secret_string(), + }, + MintedKey::Holder { machine } => GithubKey::Machine { + machine: machine.to_hex(), + }, + }; Ok(GithubCheckIn { - grant: minted.grant.to_secret_string(), + key, commit: commit.to_string(), fingerprint, ployz_version: super::preparation::VERSION.to_owned(), @@ -213,18 +257,71 @@ pub async fn github_check_in( }) } +const OLD_MINT_SKIPS: &str = + "GitHub builds a sealed value only with an up-to-date Ployz on the runner and your server"; + +/// The Sealed values the built Service reads, still sealed, for its Machine to open. +fn sealed_record( + deployment: &Value, + service: &ServiceName, +) -> Result, RpcError> { + let intent = super::preparation::frozen_intent(deployment.clone())?; + intent + .target + .iter() + .find(|spec| spec.name == *service) + .map(SealedRecord::of) + .transpose() + .map(Option::flatten) + .map_err(|error| internal(&error.to_string())) +} + +/// Mint on the session's Machine, or `None` when GitHub can't take this build. async fn mint( session: &Session, id: &GithubBuildId, -) -> Result<(ployz_core::MachineId, ployz_core::BuildGrantMinted), RpcError> { + holder: Option, + sealed: Option, +) -> Result, RpcError> { // Inspect first: a failure after the mint would leave an unclaimed grant. - let machine = session.inspect().await?.id; + let details = session.inspect().await?; + let capabilities = details + .machine + .map(|machine| machine.runtime.capabilities) + .unwrap_or_default(); + let holder = match minting(holder, &capabilities, sealed.is_some()) { + Minting::Holder(holder) => Some(holder), + Minting::Legacy => None, + Minting::Skip => return Ok(None), + }; let minted = session .mint_build_grant(MintBuildGrantRequest { repository: repository(id)?, + holder, + sealed, }) .await?; - Ok((machine, minted)) + Ok(Some((details.id, minted))) +} + +#[derive(Debug, Eq, PartialEq)] +enum Minting { + Holder(BuildGrantId), + Legacy, + Skip, +} + +// The one place an old Action or an old Machine is told apart. Delete it, with +// `Minting::Legacy` and `Skip`, once every Machine advertises the capability and +// every Action sends a holder. +fn minting(holder: Option, capabilities: &BTreeSet, sealed: bool) -> Minting { + match holder { + Some(holder) if capabilities.contains(BUILD_GRANT_HOLDER_CAPABILITY) => { + Minting::Holder(holder) + } + _ if sealed => Minting::Skip, + _ => Minting::Legacy, + } } /// The one repository a runner's Build Grant may push into. @@ -341,11 +438,15 @@ pub async fn github_finish( timed_out: bool, connections: Vec, ) -> Result { - let build = store_call(&store, { + let build = match store_call(&store, { let id = id.clone(); move |store| store.github_build(&id) }) - .await?; + .await? + { + GithubHandOff::Held(build) => *build, + GithubHandOff::PassedOn { .. } => return Ok(GithubFinish::Ended(BuildStatus::Pending)), + }; if build.status != BuildStatus::Building { return Ok(GithubFinish::Ended(build.status)); } @@ -530,6 +631,44 @@ fn conflict(message: &str) -> RpcError { mod tests { use super::*; + #[test] + fn only_a_sealed_build_without_a_holder_mint_skips_github() { + let holder = BuildGrantId::parse("d".repeat(64)).unwrap(); + let new = BTreeSet::from([BUILD_GRANT_HOLDER_CAPABILITY.to_owned()]); + let old = BTreeSet::new(); + for sealed in [false, true] { + assert_eq!(minting(Some(holder), &new, sealed), Minting::Holder(holder)); + } + for (holder, capabilities) in [(Some(holder), &old), (None, &new), (None, &old)] { + assert_eq!(minting(holder, capabilities, false), Minting::Legacy); + assert_eq!(minting(holder, capabilities, true), Minting::Skip); + } + } + + #[test] + fn a_check_in_reads_as_the_grant_or_the_machine_to_dial() { + let check_in = |key| GithubCheckIn { + key, + commit: "a".repeat(40), + fingerprint: "f".into(), + ployz_version: "1".into(), + deployment: Value::Null, + }; + let legacy = serde_json::to_value(check_in(GithubKey::Grant { + grant: "ployzgrant1:x".into(), + })) + .unwrap(); + assert_eq!(legacy.get("grant"), Some(&Value::from("ployzgrant1:x"))); + assert!(legacy.get("machine").is_none()); + assert_eq!(legacy.get("ployzVersion"), Some(&Value::from("1"))); + let holder = serde_json::to_value(check_in(GithubKey::Machine { + machine: "0".repeat(64), + })) + .unwrap(); + assert_eq!(holder.get("machine"), Some(&Value::from("0".repeat(64)))); + assert!(holder.get("grant").is_none()); + } + #[test] fn a_machine_that_cannot_read_grants_fails_the_build_by_name() { let machine = ployz_core::MachineId::random(); diff --git a/core/crates/ployz/src/sdk/payloads.rs b/core/crates/ployz/src/sdk/payloads.rs index 604e8c152a..c2b7b16be7 100644 --- a/core/crates/ployz/src/sdk/payloads.rs +++ b/core/crates/ployz/src/sdk/payloads.rs @@ -131,6 +131,7 @@ fn typescript_declarations() -> String { declarations.add::(); declarations.add::(); declarations.add::(); + declarations.add::(); declarations.add::(); declarations.add::(); declarations.add::(); diff --git a/core/crates/ployz/src/sdk/preparation.rs b/core/crates/ployz/src/sdk/preparation.rs index 33ba63782b..ab389048c4 100644 --- a/core/crates/ployz/src/sdk/preparation.rs +++ b/core/crates/ployz/src/sdk/preparation.rs @@ -160,7 +160,15 @@ fn invalid(message: impl ToString) -> RpcError { } /// Capture authorized checkouts as Builds. -pub(crate) fn capture(mut input: PreparationInput) -> Result { +pub(crate) fn capture(input: PreparationInput) -> Result { + capture_opened(input, None) +} + +/// [`capture`] for a runner that already holds the Build's Sealed values `opened`. +pub(crate) fn capture_opened( + mut input: PreparationInput, + opened: Option, +) -> Result { for receipt in input.build_receipts.values().flatten() { if !ployz_core::is_lower_hex(&receipt.fingerprint, 64) || !receipt @@ -234,7 +242,7 @@ pub(crate) fn capture(mut input: PreparationInput) -> Result Result { @@ -208,13 +207,8 @@ pub async fn select_build_machine( reasons.push("no Machines observed".into()); shown.push("no Servers observed".into()); } - let lead = sealed.then_some(SEALED_BUILDS_RUN_ON_SERVER); let message = format!("no eligible Build Machine: {}", reasons.join("; ")); - let message = match lead { - Some(lead) => format!("{lead} {message}"), - None => message, - }; - let details = serde_json::json!({"rejections": rejected, "reasons": shown, "lead": lead}); + let details = serde_json::json!({"rejections": rejected, "reasons": shown}); Err(ConnectError::Remote(RpcError { code: RpcErrorCode::Unsupported, message, @@ -224,7 +218,7 @@ pub async fn select_build_machine( } use crate::{ - build::{BuiltService, CapturedBuild, SEALED_BUILDS_RUN_ON_SERVER}, + build::{BuiltService, CapturedBuild}, connect::Client, deploy::{DeployError, DeployPlan}, }; @@ -412,20 +406,12 @@ pub(super) async fn build_images( if targets.is_empty() { return Ok(builds); } - let sealed = build.reads_sealed(); - let builders = - crate::sealed_compat::builders(&machines, sealed).map_err(PreparationError::Selection)?; + let builders = crate::sealed_compat::builders(&machines, build.reads_sealed()) + .map_err(PreparationError::Selection)?; let selected = read(cancellation, async { - select_build_machine( - client, - &targets, - &builders, - sealed, - preference, - cancellation, - ) - .await - .map_err(PreparationError::Selection) + select_build_machine(client, &targets, &builders, preference, cancellation) + .await + .map_err(PreparationError::Selection) }) .await?; let id = selected.machine.id; diff --git a/core/crates/ployz/src/sdk/store_runner.rs b/core/crates/ployz/src/sdk/store_runner.rs index 0551c8d37f..beee6140a5 100644 --- a/core/crates/ployz/src/sdk/store_runner.rs +++ b/core/crates/ployz/src/sdk/store_runner.rs @@ -1342,10 +1342,7 @@ fn targets( _ => {} } if !source.builders.contains(&Builder::Servers) { - let why = match source.sealed { - Some(_) => crate::build::SEALED_BUILDS_RUN_ON_SERVER, - None => source.message.as_deref().unwrap_or_default(), - }; + let why = source.message.as_deref().unwrap_or_default(); failed.push(format!( "{}: {}", source.service, diff --git a/core/crates/ployz/src/sdk/store_runner_tests.rs b/core/crates/ployz/src/sdk/store_runner_tests.rs index c59fb747fa..a11602e624 100644 --- a/core/crates/ployz/src/sdk/store_runner_tests.rs +++ b/core/crates/ployz/src/sdk/store_runner_tests.rs @@ -803,7 +803,7 @@ fn separately_uploaded() -> Claimed { } #[test] -fn a_sealed_build_with_no_server_in_its_walk_fails_saying_it_runs_on_a_server() { +fn a_build_with_no_server_in_its_walk_fails_with_the_reason_github_gave() { let mut claimed = separately_uploaded(); claimed.sources.push( serde_json::from_value(serde_json::json!({ @@ -817,7 +817,6 @@ fn a_sealed_build_with_no_server_in_its_walk_fails_saying_it_runs_on_a_server() "commit": "a".repeat(40), "builders": [], "preferred_machine": null, - "sealed": "NPM_TOKEN", "status": "pending", "message": "GitHub has no ployz-build.yml workflow", })) @@ -830,11 +829,11 @@ fn a_sealed_build_with_no_server_in_its_walk_fails_saying_it_runs_on_a_server() upload: Some(PathBuf::from("/extracted")), }, ) else { - panic!("a sealed build no Server takes fails"); + panic!("a build no Server takes fails"); }; assert_eq!( message, - "Build failed. api: Builds that read a sealed value run on your server. \ + "Build failed. api: GitHub has no ployz-build.yml workflow. \ No other Builder in your Build Order can take it" ); } diff --git a/core/crates/ployz/src/sealed_compat.rs b/core/crates/ployz/src/sealed_compat.rs index 9b110e9186..39eed36c85 100644 --- a/core/crates/ployz/src/sealed_compat.rs +++ b/core/crates/ployz/src/sealed_compat.rs @@ -9,7 +9,7 @@ use ployz_core::{ RunTarget, machine_matches_target, }; -use crate::{build::SEALED_BUILDS_RUN_ON_SERVER, cluster::Client, connect::ConnectError}; +use crate::{cluster::Client, connect::ConnectError}; const SEALED_READY: [&str; 2] = [MANAGEMENT_SCOPE_CAPABILITY, ENV_SEALED_CAPABILITY]; @@ -98,11 +98,10 @@ pub(crate) fn builders( names.join(", ") ) }; - let mut error = unsupported(format!("{SEALED_BUILDS_RUN_ON_SERVER} {reason}")); + let mut error = unsupported(reason.clone()); error.details = serde_json::json!({ "rejections": {"cannot open sealed values": behind.len()}, "reasons": [reason], - "lead": SEALED_BUILDS_RUN_ON_SERVER, }); Err(ConnectError::Remote(error)) } diff --git a/core/crates/ployz/src/sealed_compat_tests.rs b/core/crates/ployz/src/sealed_compat_tests.rs index 02f3143ddd..a4d283fa6e 100644 --- a/core/crates/ployz/src/sealed_compat_tests.rs +++ b/core/crates/ployz/src/sealed_compat_tests.rs @@ -233,7 +233,7 @@ fn a_build_reading_a_sealed_value_runs_only_where_the_daemon_opens_them() { assert_eq!(error.code, RpcErrorCode::Unsupported); assert_eq!( error.message, - "Builds that read a sealed value run on your server. behind cannot open sealed values; upgrade every Server first." + "behind cannot open sealed values; upgrade every Server first." ); assert_eq!( error @@ -241,12 +241,4 @@ fn a_build_reading_a_sealed_value_runs_only_where_the_daemon_opens_them() { .pointer("/rejections/cannot open sealed values"), Some(&serde_json::json!(1)) ); - - let nobody = crate::sealed_compat::builders(&[], true).unwrap_err(); - assert!( - nobody - .to_string() - .starts_with("Builds that read a sealed value run on your server."), - "{nobody}" - ); } diff --git a/core/crates/ployz/tests/build_layer3.rs b/core/crates/ployz/tests/build_layer3.rs index 08042ff56b..4e4643a32f 100644 --- a/core/crates/ployz/tests/build_layer3.rs +++ b/core/crates/ployz/tests/build_layer3.rs @@ -209,6 +209,7 @@ fn capture(root: &Path, image: &str, variables: Value, recipe: Recipe) -> Captur recipe, }, )]), + None, ) .unwrap() } diff --git a/core/crates/ployz/tests/build_layer3/grant.rs b/core/crates/ployz/tests/build_layer3/grant.rs index d2d54cb19a..b835c1aded 100644 --- a/core/crates/ployz/tests/build_layer3/grant.rs +++ b/core/crates/ployz/tests/build_layer3/grant.rs @@ -59,9 +59,14 @@ async fn ployz_build_pushes_into_a_machine_with_a_build_grant() { let minted = session .mint_build_grant(MintBuildGrantRequest { repository: ployz_core::BuildGrantRepository::parse("ployz-build/app").unwrap(), + holder: None, + sealed: None, }) .await .unwrap(); + let ployz_core::MintedKey::Grant { grant } = minted.key else { + panic!("a holderless mint returns its grant"); + }; let output = cluster .machine_shell( 0, @@ -70,7 +75,7 @@ async fn ployz_build_pushes_into_a_machine_with_a_build_grant() { PLOYZ_BUILD_GRANT={} ployz build --deployment /tmp/deployment.json \ --commit {commit} --fingerprint {fingerprint} --source /tmp/app", shell_words::quote(&deployment.to_string()), - shell_words::quote(&minted.grant.to_secret_string()), + shell_words::quote(&grant.to_secret_string()), ), ) .unwrap(); diff --git a/core/crates/ployzd/src/build.rs b/core/crates/ployzd/src/build.rs index 2c4a613d30..ad8654ed60 100644 --- a/core/crates/ployzd/src/build.rs +++ b/core/crates/ployzd/src/build.rs @@ -11,7 +11,6 @@ use ployz_build::{ }; use ployz_core::{BuildConcurrency, MachineId, OpaquePayload}; use std::{ - collections::BTreeMap, path::{Path, PathBuf}, sync::{Arc, Mutex}, time::Duration, @@ -376,7 +375,10 @@ fn receive_and_execute( let sealed = match upload.private_file(sealed::RECORD) { Ok(None) => None, Ok(Some(record)) => match sealed::open(&record, sealing_key_dir) { - Ok(opened) => Some(opened), + Ok(opened) => { + let scrub = ployz_build::OutputScrub::new(&opened); + Some((opened, scrub)) + } Err(message) => return failed(Stage::Preparation, message), }, Err(error) => return failed(Stage::Upload, ployz_core::error_chain::inline(&error)), @@ -406,7 +408,7 @@ fn receive_and_execute( } }; let progress = |progress| match &sealed { - Some(opened) => opened.scrub.progress(progress).into_iter().for_each(&send), + Some((_, scrub)) => scrub.progress(progress).into_iter().for_each(&send), None => send(progress), }; progress(Progress::Stage(Stage::Upload)); @@ -427,16 +429,15 @@ fn receive_and_execute( context.source.port = proxy.port; proxies.push(proxy); } - let none = BTreeMap::new(); let result = upload.execute( &definition, Some(&policy.docker), - sealed.as_ref().map_or(&none, |opened| &opened.values), + sealed.as_ref().map(|(opened, _)| opened), &progress, ); // Reached however the Build ended, so a cancelled one releases no held text. - if let Some(opened) = &sealed { - opened.scrub.finish().into_iter().for_each(&send); + if let Some((_, scrub)) = &sealed { + scrub.finish().into_iter().for_each(&send); } let outcome = match result { Ok(completed) => { @@ -458,7 +459,7 @@ fn receive_and_execute( Err(error) => failure(error.stage(), error), }; match &sealed { - Some(opened) => opened.scrub.outcome(outcome), + Some((_, scrub)) => scrub.outcome(outcome), None => outcome, } } diff --git a/core/crates/ployzd/src/build/sealed.rs b/core/crates/ployzd/src/build/sealed.rs index 9841563463..af4f03725b 100644 --- a/core/crates/ployzd/src/build/sealed.rs +++ b/core/crates/ployzd/src/build/sealed.rs @@ -1,487 +1,20 @@ -//! Opening a Build's Sealed values on this Machine and hiding them in -//! everything the Build reports back. +//! Opening a Build's Sealed values on this Machine. -use std::{collections::BTreeMap, path::Path, sync::Mutex}; +use std::path::Path; -use ployz_build::{Progress, remote::Outcome}; -use ployz_core::scrub::{Channel, FrameScrub, SealedForms}; +use ployz_core::OpenedSealed; -use crate::sealed_env::SealedRecord; +use crate::sealed_env::{SealedRecord, Unseal as _}; /// The private file a capture writes its Sealed values to, still sealed. pub(crate) const RECORD: &str = "sealed.json"; -pub(crate) struct Opened { - pub(crate) values: BTreeMap, - pub(crate) scrub: OutputScrub, -} - /// Open the record with this Machine's Sealing Key. The message names a /// variable at most, never what it holds. -pub(crate) fn open(record: &[u8], key_dir: Option<&Path>) -> Result { +pub(crate) fn open(record: &[u8], key_dir: Option<&Path>) -> Result { let record: SealedRecord = serde_json::from_slice(record) .map_err(|_| "the Build's sealed values are not a valid record".to_owned())?; - let (values, forms) = record + record .open_for_build(key_dir) - .map_err(|error| error.to_string())?; - Ok(Opened { - values, - scrub: OutputScrub::new(forms), - }) -} - -pub(crate) struct OutputScrub { - forms: SealedForms, - frames: Mutex, -} - -/// One stream scrubber for the unattributed output and one per step, since a -/// value never continues from one step's output into another's. A step's lives -/// until the Build ends, since BuildKit can report a step complete before its -/// last log. -#[derive(Default)] -struct Frames { - output: FrameScrub, - steps: Vec<(String, FrameScrub)>, -} - -const fn channel(stderr: bool) -> Channel { - if stderr { - Channel::Stderr - } else { - Channel::Stdout - } -} - -impl OutputScrub { - pub(crate) fn new(forms: SealedForms) -> Self { - Self { - forms, - frames: Mutex::new(Frames::default()), - } - } - - /// What of `progress` can be reported now, in order. Output is held back - /// where it could begin a value, so one event may become none. - pub(crate) fn progress(&self, progress: Progress) -> Vec { - let mut redacted = 0; - match progress { - Progress::Output(bytes) => { - let shown = self - .frames - .lock() - .expect("Build output scrub lock") - .output - .push(Channel::Stderr, &bytes, &self.forms); - if shown.is_empty() { - Vec::new() - } else { - vec![Progress::Output(shown)] - } - } - Progress::Step(mut step) => { - step.name = self.forms.scrub(&step.name, &mut redacted); - step.error = step - .error - .map(|error| self.forms.scrub(&error, &mut redacted)); - vec![Progress::Step(step)] - } - Progress::StepOutput { step, stderr, text } => { - let steps = &mut self.frames.lock().expect("Build output scrub lock").steps; - let index = steps - .iter() - .position(|(id, _)| id == &step) - .unwrap_or_else(|| { - steps.push((step.clone(), FrameScrub::default())); - steps.len() - 1 - }); - let shown = steps - .get_mut(index) - .expect("step was found or inserted") - .1 - .push(channel(stderr), text.as_bytes(), &self.forms); - if shown.is_empty() { - Vec::new() - } else { - vec![Progress::StepOutput { - step, - stderr, - text: String::from_utf8_lossy(&shown).into_owned(), - }] - } - } - other @ (Progress::Stage(_) | Progress::Timing { .. } | Progress::Target { .. }) => { - vec![other] - } - } - } - - /// What every stream still held when the Build ended, step by step and - /// then the unattributed output, with any trailing start of a value - /// hidden. A Build that ends early, cancelled or failed, reaches this the - /// same way as one that completes. - pub(crate) fn finish(&self) -> Vec { - let frames = std::mem::take(&mut *self.frames.lock().expect("Build output scrub lock")); - let mut events: Vec = frames - .steps - .into_iter() - .flat_map(|(step, frames)| { - frames - .finish(&self.forms) - .into_iter() - .map(move |(channel, bytes)| Progress::StepOutput { - step: step.clone(), - stderr: matches!(channel, Channel::Stderr), - text: String::from_utf8_lossy(&bytes).into_owned(), - }) - }) - .collect(); - let tail: Vec = frames - .output - .finish(&self.forms) - .into_iter() - .flat_map(|(_, bytes)| bytes) - .collect(); - if !tail.is_empty() { - events.push(Progress::Output(tail)); - } - events - } - - pub(crate) fn outcome(&self, outcome: Outcome) -> Outcome { - let mut redacted = 0; - match outcome { - Outcome::Failed { - work, - stage, - message, - } => Outcome::Failed { - work, - stage, - message: self.forms.scrub(&message, &mut redacted), - }, - Outcome::Unknown { - work, - stage, - message, - } => Outcome::Unknown { - work, - stage, - message: self.forms.scrub(&message, &mut redacted), - }, - other @ (Outcome::Images { .. } - | Outcome::Validated { .. } - | Outcome::Published { .. } - | Outcome::CapabilitiesChecked { .. }) => other, - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use ployz_build::BuildStep; - use ployz_core::scrub::SealedValues; - - const STEP: &str = "sha256:step-one"; - - fn scrub(value: &str) -> OutputScrub { - let values: SealedValues = [("NPM_TOKEN".to_owned(), value.to_owned())] - .into_iter() - .collect(); - OutputScrub::new(SealedForms::compile(&values)) - } - - fn step_output(text: &str) -> Progress { - Progress::StepOutput { - step: STEP.into(), - stderr: false, - text: text.into(), - } - } - - fn completed() -> Progress { - Progress::Step(BuildStep { - id: STEP.into(), - name: "RUN npm install".into(), - started: Some("t0".into()), - completed: Some("t1".into()), - cached: false, - error: None, - }) - } - - /// Every event's text in order, with the shown step output joined. - fn report(scrub: &OutputScrub, events: Vec) -> (String, Vec) { - let mut shown = Vec::new(); - for event in events { - shown.extend(scrub.progress(event)); - } - shown.extend(scrub.finish()); - let text = shown - .iter() - .filter_map(|event| { - if let Progress::StepOutput { text, .. } = event { - Some(text.as_str()) - } else { - None - } - }) - .collect(); - (text, shown) - } - - #[test] - fn a_value_cut_between_two_step_output_events_never_shows() { - let value = "opened-token-0123456789"; - let scrub = scrub(value); - let padding = "x".repeat(64 * 1024 - 7); - let first = format!("{padding}{}", &value[..7]); - let second = format!("{} and more\n", &value[7..]); - let (text, shown) = report( - &scrub, - vec![step_output(&first), step_output(&second), completed()], - ); - assert!(!text.contains(value), "{text}"); - assert_eq!(text, format!("{padding}[secret NPM_TOKEN] and more\n")); - assert!(matches!(shown.last(), Some(Progress::Step(step)) if step.completed.is_some())); - } - - #[test] - fn a_multiline_value_cut_at_its_own_newline_never_shows() { - let value = "line1-secret\nline2-secret"; - let scrub = scrub(value); - let (text, _) = report( - &scrub, - vec![ - step_output("cert: line1-secret\n"), - step_output("line2-secret end\n"), - completed(), - ], - ); - assert!(!text.contains("line1-secret"), "{text}"); - assert!(!text.contains("line2-secret"), "{text}"); - assert_eq!(text, "cert: [secret NPM_TOKEN] end\n"); - } - - #[test] - fn plain_output_arrives_whole_and_in_order_and_a_held_tail_leaves_at_the_end() { - let scrub = scrub("opened-token-0123456789"); - let events = vec![ - step_output("one\n"), - step_output("two "), - step_output("three\n"), - Progress::StepOutput { - step: "sha256:step-two".into(), - stderr: true, - text: "warning: slow".into(), - }, - step_output("four"), - ]; - let (text, shown) = report(&scrub, events); - assert_eq!(text, "one\ntwo three\nfourwarning: slow"); - let order: Vec<(&str, bool)> = shown - .iter() - .filter_map(|event| { - if let Progress::StepOutput { step, stderr, .. } = event { - Some((step.as_str(), *stderr)) - } else { - None - } - }) - .collect(); - // Held text leaves as late as it must: "two " waits for its line end, and - // what each step still holds leaves at the end, step by step. - assert_eq!( - order, - [ - (STEP, false), - (STEP, false), - (STEP, false), - ("sha256:step-two", true) - ] - ); - } - - #[test] - fn held_step_tails_follow_first_output_order_instead_of_digest_order() { - let output = |step: &str, text: &str| Progress::StepOutput { - step: step.into(), - stderr: false, - text: text.into(), - }; - let (text, shown) = report( - &scrub("opened-token-0123456789"), - vec![ - output("sha256:z-first", "first "), - output("sha256:a-second", "second"), - output("sha256:z-first", "tail"), - ], - ); - assert_eq!(text, "first tailsecond"); - let steps: Vec<_> = shown - .iter() - .filter_map(|event| { - if let Progress::StepOutput { step, .. } = event { - Some(step.as_str()) - } else { - None - } - }) - .collect(); - assert_eq!(steps, ["sha256:z-first", "sha256:a-second"]); - } - - /// A step's last log can arrive after BuildKit reports it complete, so a - /// value cut by the completion is still one value. - #[test] - fn a_value_cut_by_its_steps_completion_never_shows() { - for (value, before, after, expected) in [ - ( - "opened-token-0123456789", - "token is opened-", - "token-0123456789 and more\n", - "token is [secret NPM_TOKEN] and more\n", - ), - ( - "s3cr", - "NPM_TOKEN=", - "s3cr\n", - "NPM_TOKEN=[secret NPM_TOKEN]\n", - ), - ] { - let scrub = scrub(value); - let (text, shown) = report( - &scrub, - vec![step_output(before), completed(), step_output(after)], - ); - assert!(!text.contains(value), "{text}"); - assert_eq!(text, expected); - assert!( - shown - .iter() - .any(|event| matches!(event, Progress::Step(step) if step.completed.is_some())) - ); - } - } - - #[test] - fn a_character_split_by_parser_completion_still_scrubs_the_whole_value() { - use base64::Engine as _; - use ployz_build::{Admission, HostPolicy, Output, Request, Target, execute_admitted}; - use std::{fs, os::unix::fs::PermissionsExt as _}; - - let value = "🐓opened-token-0123456789"; - let (head, tail) = value.as_bytes().split_at(2); - let record = |bytes: &[u8]| { - format!( - "{{\"logs\":[{{\"vertex\":\"{STEP}\",\"stream\":1,\"data\":\"{}\"}}]}}\n", - base64::engine::general_purpose::STANDARD.encode(bytes) - ) - }; - for sealed in [false, true] { - let root = tempfile::tempdir().unwrap(); - fs::set_permissions(root.path(), fs::Permissions::from_mode(0o700)).unwrap(); - let docker = root.path().join("docker"); - fs::write( - &docker, - format!( - r#"#!/bin/sh -case "$1 $2" in - 'context show') echo default ;; - 'info --format') echo '{{"OSType":"linux","CgroupVersion":"2","Architecture":"amd64","DriverStatus":[["driver-type","io.containerd.snapshotter.v1"]]}}' ;; - 'buildx ls') echo '{{"Name":"{}","Nodes":[{{"Status":"running","Platforms":["linux/amd64"]}}]}}' ;; - 'buildx bake') cat solve-events ;; -esac -exit 0 -"#, - ployz_build::builder_name() - ), - ) - .unwrap(); - fs::set_permissions(&docker, fs::Permissions::from_mode(0o700)).unwrap(); - fs::write( - root.path().join("solve-events"), - format!( - "{}{{\"vertexes\":[{{\"digest\":\"{STEP}\",\"completed\":\"t1\"}}]}}\n{}", - record(head), - record(&[tail, b"\n"].concat()) - ), - ) - .unwrap(); - let admission = Admission::try_acquire_with(&HostPolicy { - state_directory: root.path().to_owned(), - configuration_file: root.path().join("build.yaml"), - docker: docker.clone(), - ..Default::default() - }) - .unwrap(); - let request = Request { - image_contexts: &BTreeMap::new(), - compose_file: &root.path().join("compose.yaml"), - sealed_override: None, - working_dir: root.path(), - environment: &BTreeMap::new(), - docker: Some(&docker), - targets: &[Target { - name: "app".into(), - platforms: Vec::new(), - }], - railpack: &[], - build_args: &[], - output: Output::Validate, - no_cache: false, - pull: false, - sealed, - }; - let events = Mutex::new(Vec::new()); - execute_admitted(&request, admission, &|event| { - events.lock().unwrap().push(event); - }) - .unwrap(); - let events = events.into_inner().unwrap(); - if !sealed { - let text: Vec<&str> = events - .iter() - .filter_map(|event| { - if let Progress::StepOutput { text, .. } = event { - Some(text.as_str()) - } else { - None - } - }) - .collect(); - assert_eq!( - text, - ["\u{FFFD}", "\u{FFFD}\u{FFFD}opened-token-0123456789\n"] - ); - continue; - } - let (text, _) = report(&scrub(value), events); - assert!(!text.contains(value), "{text}"); - assert!(!text.contains("opened-token-0123456789"), "{text}"); - assert_eq!(text, "[secret NPM_TOKEN]\n"); - } - } - - #[test] - fn plain_text_held_across_a_steps_completion_shows_whole() { - // The value starts with the byte `Done` ends with, so a stream ended at - // the completion would have hidden it as the start of the value. - let scrub = scrub("e-opened-token-0123456789"); - let (text, _) = report( - &scrub, - vec![step_output("Done"), completed(), step_output(" in 1.2s\n")], - ); - assert_eq!(text, "Done in 1.2s\n"); - } - - #[test] - fn a_value_begun_when_the_build_stops_stays_hidden() { - let value = "opened-token-0123456789"; - let scrub = scrub(value); - let (text, _) = report(&scrub, vec![step_output("token is opened-tok")]); - assert!(!text.contains("opened-tok"), "{text}"); - assert_eq!(text, "token is [secret NPM_TOKEN]"); - } + .map_err(|error| error.to_string()) } diff --git a/core/crates/ployzd/src/build/tests.rs b/core/crates/ployzd/src/build/tests.rs index 37d67cc54a..08873b72d0 100644 --- a/core/crates/ployzd/src/build/tests.rs +++ b/core/crates/ployzd/src/build/tests.rs @@ -218,6 +218,7 @@ impl Fixture { }, )] .into(), + None, ) .unwrap() } diff --git a/core/crates/ployzd/src/docker/mod.rs b/core/crates/ployzd/src/docker/mod.rs index e42cac8c6a..a625993029 100644 --- a/core/crates/ployzd/src/docker/mod.rs +++ b/core/crates/ployzd/src/docker/mod.rs @@ -846,7 +846,7 @@ impl ContainerRuntime { .as_ref() .and_then(|config| config.labels.as_ref()) .and_then(|labels| labels.get(crate::sealed_env::LABEL_SEALED)); - Ok(crate::sealed_env::SealedRecord::from_label( + Ok(crate::sealed_env::record_from_label( label.map(String::as_str), )?) } @@ -920,7 +920,7 @@ impl ContainerRuntime { let mut last_check = last_docker_check(inspected.state.as_ref(), || { crate::sealed_env::Lines::new( *container_id, - crate::sealed_env::SealedRecord::from_label( + crate::sealed_env::record_from_label( labels .get(crate::sealed_env::LABEL_SEALED) .map(String::as_str), diff --git a/core/crates/ployzd/src/docker/stream.rs b/core/crates/ployzd/src/docker/stream.rs index 4316b7cca2..814891b3e6 100644 --- a/core/crates/ployzd/src/docker/stream.rs +++ b/core/crates/ployzd/src/docker/stream.rs @@ -20,7 +20,7 @@ use super::{ContainerRuntime, docker_error}; use crate::logs::{ JournalError, LogLineStream, LogSource, RawLogEntry, RpcStream, serve_logs, split_at_space, }; -use crate::sealed_env::{self, SealedRecord}; +use crate::sealed_env::{self, Unseal as _}; const NANOS_PER_SECOND: i64 = 1_000_000_000; const EXEC_OUTPUT_IDLE: Duration = Duration::from_millis(100); @@ -169,7 +169,7 @@ impl ContainerRuntime { machine_id: *machine_id, machine_name: machine_name.clone(), }; - let record = SealedRecord::from_label( + let record = sealed_env::record_from_label( observation .labels .get(sealed_env::LABEL_SEALED) diff --git a/core/crates/ployzd/src/machine_api/local.rs b/core/crates/ployzd/src/machine_api/local.rs index 5abc7c6412..5121255fa0 100644 --- a/core/crates/ployzd/src/machine_api/local.rs +++ b/core/crates/ployzd/src/machine_api/local.rs @@ -827,15 +827,24 @@ impl MachineRpc for MachineService { request: Request, ) -> Result, Status> { let request = expect::(request)?; + let key_dir = self + .containers() + .ok() + .and_then(|containers| containers.sealing_key_dir()); + let mint = match crate::management::build_grant::Mint::check(request, key_dir) { + Ok(mint) => mint, + Err(refused) => return respond(refused), + }; match crate::management::build_grant::mint_for( &self.grants, &self.local, Arc::clone(&self.ingest), - request, + mint, ) .await { - Ok(minted) => respond(minted), + Ok(Ok(minted)) => respond(minted), + Ok(Err(refused)) => respond(refused), Err(error) => local_error(error), } } diff --git a/core/crates/ployzd/src/management/build_grant.rs b/core/crates/ployzd/src/management/build_grant.rs index 9e98d5f68a..c214ca2f67 100644 --- a/core/crates/ployzd/src/management/build_grant.rs +++ b/core/crates/ployzd/src/management/build_grant.rs @@ -8,11 +8,15 @@ //! already holds that tag at its digest), after [`GRANT_LIFETIME`], or when the daemon //! stops. The Machine RPC handlers only forward to [`mint_for`] and //! [`BuildGrants::inspect`]. +//! +//! A grant minted for a holder can carry the built Service's Sealed values, still +//! sealed. Its holder reads them opened from `GET /ployz/sealed` once, before the push. use std::{ collections::HashMap, convert::Infallible, net::SocketAddr, + path::{Path, PathBuf}, sync::{Arc, Mutex}, time::Duration, }; @@ -24,18 +28,20 @@ use http_body_util::BodyExt as _; use hyper::{body::Incoming, server::conn::http1, service::service_fn}; use hyper_util::rt::TokioIo; use iroh::{ - SecretKey, + PublicKey, SecretKey, endpoint::{Connection, VarInt}, }; use ployz_core::{ BUILD_GRANT_ENDED, BUILD_GRANT_REFUSED, BuildGrant, BuildGrantId, BuildGrantInspected, BuildGrantMinted, BuildGrantRepository, ImageDigest, ManagementIdentity, MintBuildGrantRequest, - RETAINED_DIGEST_TAG_PREFIX, RpcError, RpcErrorCode, + MintedKey, RETAINED_DIGEST_TAG_PREFIX, RpcError, RpcErrorCode, SealedRecord, }; use sha2::{Digest as _, Sha256}; use tokio::time::Instant; use tokio_util::sync::CancellationToken; +use crate::sealed_env::Unseal as _; + /// How long a grant lives. Ployz Cloud mints it at /// check-in and gives the run 2h from there (`GITHUB_RUN_BUDGET_MS` in the dashboard's /// `github-image-builds.server.ts`), so this must outlive that budget: a run Cloud @@ -56,6 +62,21 @@ struct Grant { ingest: SocketAddr, expires: Instant, push: Mutex, + /// Locked only under `push`. + sealed: Mutex, +} + +enum Sealing { + None, + Sealed(Sealed), + /// Its holder read them; nothing more is served. + Served, +} + +/// A record this Machine opened at mint, kept sealed until its holder reads it. +struct Sealed { + record: SealedRecord, + key_dir: PathBuf, } enum Push { @@ -66,30 +87,117 @@ enum Push { Pushed(ImageDigest), } +/// A mint request this Machine accepts, not yet minted. +pub struct Mint { + repository: BuildGrantRepository, + key: Key, + sealed: Option, +} + +enum Key { + /// This Machine made the key; the minted grant returns its secret. + Made(SecretKey), + /// The holder made it and keeps the secret. + Holder(PublicKey), +} + +impl Mint { + /// Check `request` before anything is opened for it, and that its Sealed values + /// open with the Sealing Key in `key_dir`. + /// + /// # Errors + /// `InvalidArgument` when the holder is not a public key or Sealed values have no + /// holder; `Conflict` when this Machine has no Sealing Key or they don't open here. + pub fn check(request: MintBuildGrantRequest, key_dir: Option<&Path>) -> Result { + let key = match request.holder { + None => Key::Made(SecretKey::generate()), + Some(holder) => Key::Holder( + hex::decode(holder.as_str()) + .ok() + .and_then(|bytes| <[u8; 32]>::try_from(bytes).ok()) + .and_then(|bytes| PublicKey::from_bytes(&bytes).ok()) + .ok_or_else(|| { + refused( + RpcErrorCode::InvalidArgument, + "the Build Grant holder is not a public key".into(), + ) + })?, + ), + }; + let sealed = match request.sealed { + None => None, + Some(_) if matches!(key, Key::Made(_)) => { + return Err(refused( + RpcErrorCode::InvalidArgument, + "a Build Grant carrying Sealed values needs a holder".into(), + )); + } + Some(record) => { + let key_dir = key_dir.ok_or_else(|| { + refused( + RpcErrorCode::Conflict, + "this Machine has no Sealing Key to open the Build's Sealed values".into(), + ) + })?; + record + .open_for_build(Some(key_dir)) + .map_err(|error| refused(RpcErrorCode::Conflict, error.to_string()))?; + Some(Sealed { + record, + key_dir: key_dir.to_owned(), + }) + } + }; + Ok(Mint { + repository: request.repository, + key, + sealed, + }) + } +} + impl BuildGrants { - /// Mint a grant for one push into `repository` through the ingest at `ingest`. + /// Mint a grant for one push into `mint`'s repository through the ingest at + /// `ingest`, dialed at `machine`. + /// + /// # Errors + /// `Conflict` when the holder already holds a live grant. pub fn mint( &self, machine: ManagementIdentity, - repository: BuildGrantRepository, + mint: Mint, ingest: SocketAddr, - ) -> BuildGrantMinted { - let secret = SecretKey::generate(); - let id = grant_id(secret.public().as_bytes()); + ) -> Result { + let (id, key) = match mint.key { + Key::Made(secret) => ( + grant_id(secret.public().as_bytes()), + MintedKey::Grant { + grant: BuildGrant::new(machine, secret.to_bytes()), + }, + ), + Key::Holder(holder) => (grant_id(holder.as_bytes()), MintedKey::Holder { machine }), + }; let grant = Arc::new(Grant { - repository, + repository: mint.repository, ingest, expires: Instant::now() + GRANT_LIFETIME, push: Mutex::new(Push::Open), + sealed: Mutex::new(mint.sealed.map_or(Sealing::None, Sealing::Sealed)), }); let mut grants = self.grants.lock().expect("grant registry lock"); grants.retain(|_, grant| grant.expires > Instant::now()); + if grants.contains_key(&id) { + return Err(refused( + RpcErrorCode::Conflict, + "this holder already holds a live Build Grant".into(), + )); + } grants.insert(id, grant); - BuildGrantMinted { + Ok(BuildGrantMinted { id, - grant: BuildGrant::new(machine, secret.to_bytes()), + key, expires_in_seconds: GRANT_LIFETIME.as_secs(), - } + }) } /// Report what a live grant pushed. Reading does not spend or end it. @@ -143,16 +251,25 @@ pub(crate) async fn mint_for( grants: &BuildGrants, local: &crate::machine::LocalMachine, ingest: Arc, - request: MintBuildGrantRequest, -) -> Result { + mint: Mint, +) -> Result, crate::machine::LocalMachineError> { let opened = local.ensure_image_ingest(ingest).await?.destination; Ok(grants.mint( local.record().management_secret().public_key(), - request.repository, + mint, SocketAddr::from((opened.management_address.0, opened.port)), )) } +const fn refused(code: RpcErrorCode, message: String) -> RpcError { + RpcError { + code, + message, + details: serde_json::Value::Null, + cause: Vec::new(), + } +} + fn grant_id(key: &[u8; 32]) -> BuildGrantId { BuildGrantId::parse(hex::encode(key)).expect("hex of 32 bytes is a Build Grant ID") } @@ -210,9 +327,10 @@ pub(super) async fn serve( connection.close(VarInt::from_u32(BUILD_GRANT_ENDED), b"build grant ended"); } -/// One registry call a grant allows, in its one repository. +/// One call a grant allows. #[derive(Debug, Eq, PartialEq)] enum Route { + Sealed, Ping, Blob, Upload, @@ -233,6 +351,9 @@ impl Route { query: Option<&str>, repository: &BuildGrantRepository, ) -> Option { + if path == "/ployz/sealed" { + return (*method == Method::GET).then_some(Self::Sealed); + } if path == "/v2/" { return matches!(*method, Method::GET | Method::HEAD).then_some(Self::Ping); } @@ -297,6 +418,7 @@ async fn handle( } let (parts, body) = request.into_parts(); let (digest, tagged) = match route { + Route::Sealed => return sealed(grant), Route::Ping | Route::Blob | Route::Upload | Route::ManifestHead => { let body = reqwest::Body::wrap_stream(Body::new(body).into_data_stream()); return forward(client, grant.ingest, &parts, body).await; @@ -355,6 +477,45 @@ async fn handle( response } +/// The grant's Sealed values, opened, for its holder once, while the grant is live and +/// nothing is pushing or pushed. +fn sealed(grant: &Grant) -> Response { + let sealed = { + let push = grant.push.lock().expect("grant push lock"); + if !matches!(*push, Push::Open) || grant.expires <= Instant::now() { + return denied(StatusCode::FORBIDDEN, "the Build Grant has ended"); + } + let mut sealing = grant.sealed.lock().expect("grant sealed lock"); + match std::mem::replace(&mut *sealing, Sealing::Served) { + Sealing::Sealed(sealed) => sealed, + Sealing::Served => { + return denied( + StatusCode::FORBIDDEN, + "the Build Grant's Sealed values were already read", + ); + } + Sealing::None => { + *sealing = Sealing::None; + return denied( + StatusCode::NOT_FOUND, + "the Build Grant carries no Sealed values", + ); + } + } + }; + match sealed.record.open_for_build(Some(&sealed.key_dir)) { + Ok(opened) => Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "application/json") + .header(header::CACHE_CONTROL, "no-store") + .body(Body::from( + serde_json::to_vec(&opened).expect("opened values encode"), + )) + .expect("static response"), + Err(error) => denied(StatusCode::PRECONDITION_FAILED, &error.to_string()), + } +} + async fn forward( client: &reqwest::Client, ingest: SocketAddr, @@ -425,7 +586,229 @@ fn denied(status: StatusCode, message: &str) -> Response { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + + use ployz_core::{EnvValue, OpenedSealed, SealedScope}; + use super::*; + use crate::sealed_env::test_support::Cluster; + + fn request(holder: Option<&str>) -> MintBuildGrantRequest { + MintBuildGrantRequest { + repository: BuildGrantRepository::parse("ployz-build/web").unwrap(), + holder: holder.map(|holder| BuildGrantId::parse(holder).unwrap()), + sealed: None, + } + } + + fn mint(grants: &BuildGrants, request: MintBuildGrantRequest) -> BuildGrantMinted { + let machine = ManagementIdentity::from_bytes([7; 32]); + let ingest = SocketAddr::from(([127, 0, 0, 1], 5000)); + grants + .mint(machine, Mint::check(request, None).unwrap(), ingest) + .unwrap() + } + + const MARKER: &str = "marker-sealed-0123456789"; + + fn holder() -> PublicKey { + SecretKey::from_bytes(&[3; 32]).public() + } + + fn sealed_request(cluster: &Cluster) -> MintBuildGrantRequest { + let env = BTreeMap::from([( + "NPM_TOKEN".to_owned(), + EnvValue::Sealed(cluster.seal("NPM_TOKEN", SealedScope::Shared, MARKER)), + )]); + MintBuildGrantRequest { + sealed: Some(cluster.open(env).unwrap().record.unwrap()), + ..request(Some(&hex::encode(holder().as_bytes()))) + } + } + + fn sealed_grant(cluster: &Cluster) -> Arc { + let grants = BuildGrants::default(); + let mint = Mint::check(sealed_request(cluster), Some(&cluster.dir.0)).unwrap(); + grants + .mint( + ManagementIdentity::from_bytes([7; 32]), + mint, + SocketAddr::from(([127, 0, 0, 1], 5000)), + ) + .unwrap(); + grants.admit(holder().as_bytes()).unwrap() + } + + async fn read(response: Response) -> (StatusCode, String) { + let status = response.status(); + let body = response.into_body().collect().await.unwrap().to_bytes(); + (status, String::from_utf8(body.to_vec()).unwrap()) + } + + #[tokio::test] + async fn a_holder_reads_its_grants_sealed_values_opened() { + let cluster = Cluster::new(); + let (status, body) = read(sealed(&sealed_grant(&cluster))).await; + assert_eq!(status, StatusCode::OK); + let opened: OpenedSealed = serde_json::from_str(&body).unwrap(); + assert_eq!(opened.get("NPM_TOKEN"), Some(MARKER)); + } + + #[tokio::test] + async fn sealed_values_end_with_the_push_or_the_grant() { + let cluster = Cluster::new(); + let pushing = sealed_grant(&cluster); + *pushing.push.lock().unwrap() = Push::Pushing; + let pushed = sealed_grant(&cluster); + *pushed.push.lock().unwrap() = + Push::Pushed(ImageDigest::parse(format!("sha256:{}", "a".repeat(64))).unwrap()); + let live = sealed_grant(&cluster); + let expired = Grant { + repository: live.repository.clone(), + ingest: live.ingest, + expires: Instant::now(), + push: Mutex::new(Push::Open), + sealed: Mutex::new(Sealing::Sealed(Sealed { + record: sealed_request(&cluster).sealed.unwrap(), + key_dir: cluster.dir.0.clone(), + })), + }; + for grant in [&*pushing, &*pushed, &expired] { + let (status, body) = read(sealed(grant)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + assert!(!body.contains(MARKER)); + } + } + + #[tokio::test] + async fn a_holder_reads_sealed_values_once() { + let cluster = Cluster::new(); + let grant = sealed_grant(&cluster); + assert_eq!(read(sealed(&grant)).await.0, StatusCode::OK); + let (status, body) = read(sealed(&grant)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + assert!(!body.contains(MARKER)); + } + + #[tokio::test] + async fn sealed_values_that_fail_to_open_are_not_served_later() { + let cluster = Cluster::new(); + let elsewhere = Cluster::new(); + let grant = sealed_grant(&cluster); + *grant.sealed.lock().unwrap() = Sealing::Sealed(Sealed { + record: sealed_request(&cluster).sealed.unwrap(), + key_dir: elsewhere.dir.0.clone(), + }); + assert_eq!( + read(sealed(&grant)).await.0, + StatusCode::PRECONDITION_FAILED + ); + let (status, body) = read(sealed(&grant)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + } + + #[test] + fn a_holder_with_a_live_grant_mints_no_second() { + let grants = BuildGrants::default(); + let holder = hex::encode(holder().as_bytes()); + let first = mint(&grants, request(Some(&holder))); + let Err(refused) = grants.mint( + ManagementIdentity::from_bytes([7; 32]), + Mint::check(request(Some(&holder)), None).unwrap(), + SocketAddr::from(([127, 0, 0, 1], 5000)), + ) else { + panic!("a second mint would replace the live grant"); + }; + assert_eq!(refused.code, RpcErrorCode::Conflict); + assert!(grants.inspect(&first.id).is_ok()); + } + + #[tokio::test] + async fn a_grant_without_sealed_values_has_none_to_read() { + let grants = BuildGrants::default(); + mint(&grants, request(Some(&hex::encode(holder().as_bytes())))); + let grant = grants.admit(holder().as_bytes()).unwrap(); + assert_eq!(read(sealed(&grant)).await.0, StatusCode::NOT_FOUND); + } + + #[test] + fn sealed_values_need_a_holder_and_must_open_here() { + let cluster = Cluster::new(); + let holderless = MintBuildGrantRequest { + holder: None, + ..sealed_request(&cluster) + }; + let Err(refused) = Mint::check(holderless, Some(&cluster.dir.0)) else { + panic!("a holderless grant would return its key to Cloud"); + }; + assert_eq!(refused.code, RpcErrorCode::InvalidArgument); + + let elsewhere = Cluster::new(); + for key_dir in [Some(elsewhere.dir.0.as_path()), None] { + let Err(refused) = Mint::check(sealed_request(&cluster), key_dir) else { + panic!("another Cluster's key opens nothing"); + }; + assert_eq!(refused.code, RpcErrorCode::Conflict); + assert!(!refused.message.contains(MARKER), "{}", refused.message); + } + } + + #[test] + fn a_holder_mint_names_the_holder_and_returns_no_secret() { + let grants = BuildGrants::default(); + let holder = SecretKey::from_bytes(&[3; 32]).public(); + let minted = mint(&grants, request(Some(&hex::encode(holder.as_bytes())))); + assert_eq!(minted.id.as_str(), hex::encode(holder.as_bytes())); + assert_eq!( + minted.key, + MintedKey::Holder { + machine: ManagementIdentity::from_bytes([7; 32]) + } + ); + assert!(grants.admit(holder.as_bytes()).is_some()); + } + + #[test] + fn a_holderless_mint_returns_the_secret_it_made() { + let grants = BuildGrants::default(); + let minted = mint(&grants, request(None)); + let MintedKey::Grant { grant } = minted.key else { + panic!("a holderless mint returns its grant"); + }; + let public = SecretKey::from_bytes(grant.secret()).public(); + assert_eq!(minted.id.as_str(), hex::encode(public.as_bytes())); + assert_eq!(grant.machine(), &ManagementIdentity::from_bytes([7; 32])); + assert!(grants.admit(public.as_bytes()).is_some()); + } + + #[test] + fn a_holder_that_is_not_a_public_key_is_refused() { + let mut point = [0; 32]; + point[0] = 2; + let Err(refused) = Mint::check(request(Some(&hex::encode(point))), None) else { + panic!("y = 2 is not on the curve"); + }; + assert_eq!(refused.code, RpcErrorCode::InvalidArgument); + } + + #[test] + fn a_grant_serves_sealed_values_only_to_a_get() { + let repository = BuildGrantRepository::parse("ployz-build/web").unwrap(); + assert_eq!( + Route::parse(&Method::GET, "/ployz/sealed", None, &repository), + Some(Route::Sealed) + ); + for method in [Method::HEAD, Method::POST, Method::PUT] { + assert_eq!( + Route::parse(&method, "/ployz/sealed", None, &repository), + None + ); + } + assert_eq!( + Route::parse(&Method::GET, "/ployz/sealed/x", None, &repository), + None + ); + } #[test] fn a_grant_allows_only_push_calls_into_its_repository() { diff --git a/core/crates/ployzd/src/management/mod.rs b/core/crates/ployzd/src/management/mod.rs index 4f33ab7990..91bbccec0d 100644 --- a/core/crates/ployzd/src/management/mod.rs +++ b/core/crates/ployzd/src/management/mod.rs @@ -42,7 +42,7 @@ use crate::machine::{LocalMachine, LocalMachineRecord}; pub mod build_grant; pub mod forgotten; mod tunnel; -pub use build_grant::BuildGrants; +pub use build_grant::{BuildGrants, Mint}; pub use forgotten::Roster; /// Application close code sent when the remote key is not admitted and no tombstone holds it. diff --git a/core/crates/ployzd/src/observe/layout.rs b/core/crates/ployzd/src/observe/layout.rs index 372b9b9e15..22fbea7b29 100644 --- a/core/crates/ployzd/src/observe/layout.rs +++ b/core/crates/ployzd/src/observe/layout.rs @@ -64,7 +64,7 @@ use std::{ use ployz_core::ContainerId; use serde::{Deserialize, Serialize}; -use crate::sealed_env::SealedRecord; +use crate::sealed_env::{self, SealedRecord}; const STORE_DIR: &str = "ployz-observe"; pub const VERSION_DIR: &str = "v1"; @@ -352,7 +352,7 @@ impl RetainedEnv { /// or the Sealed label does not parse. #[must_use] pub fn from_inspected(entries: &[String], sealed_label: Option<&str>) -> Option { - let sealed = SealedRecord::from_label(sealed_label).ok()?; + let sealed = sealed_env::record_from_label(sealed_label).ok()?; let mut values = crate::docker::parse_environment(entries)?; for key in sealed.iter().flat_map(SealedRecord::keys) { values.remove(key); diff --git a/core/crates/ployzd/src/sealed_env.rs b/core/crates/ployzd/src/sealed_env.rs index 7a6654f13d..a876f1eca7 100644 --- a/core/crates/ployzd/src/sealed_env.rs +++ b/core/crates/ployzd/src/sealed_env.rs @@ -11,10 +11,9 @@ use std::{ use futures_util::StreamExt; use ployz_core::scrub::{self, Channel, FrameScrub, SealedForms, SealedValues}; use ployz_core::{ - ContainerId, EnvPart, EnvValue, HistoryRow, OpaquePayload, SealedIdentity, SealedScope, - SealedValue, + ContainerId, EnvPart, EnvValue, HistoryRow, OpaquePayload, OpenedSealed, SealedIdentity, + SealedScope, SealedValue, }; -use serde::{Deserialize, Serialize}; use thiserror::Error; use tokio::sync::mpsc; use tokio_stream::wrappers::ReceiverStream; @@ -27,13 +26,7 @@ use crate::{ pub(crate) const LABEL_SEALED: &str = "ployz.env.sealed"; -/// The Sealed values a Container was given, still sealed, keyed by the env -/// key they opened into. -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] -pub struct SealedRecord { - identity: SealedIdentity, - values: BTreeMap, -} +pub use ployz_core::SealedRecord; #[derive(Debug, Error)] pub enum OpenError { @@ -79,13 +72,10 @@ pub(crate) fn open( }); }; let identity = identity.ok_or_else(|| OpenError::MissingIdentity(first.clone()))?; - let record = SealedRecord { - identity: identity.clone(), - values: sealed, - }; - record.check_bindings()?; + let record = SealedRecord::new(identity.clone(), sealed); + check_bindings(&record)?; let key = load(key_dir)?; - for opened in record.opened(&key)? { + for opened in opened(&record, &key)? { values.insert(opened.name.to_owned(), opened.whole); } Ok(OpenedEnv { @@ -94,19 +84,25 @@ pub(crate) fn open( }) } -impl SealedRecord { - pub(crate) fn keys(&self) -> impl Iterator { - self.values.keys().map(String::as_str) - } +pub(crate) fn record_from_label( + label: Option<&str>, +) -> Result, serde_json::Error> { + label.map(serde_json::from_str).transpose() +} - pub(crate) fn from_label(label: Option<&str>) -> Result, serde_json::Error> { - label.map(serde_json::from_str).transpose() - } +/// Opening a record with this Machine's Sealing Key. +pub(crate) trait Unseal { + fn secrets(&self, key_dir: Option<&Path>) -> Result; - pub(crate) fn secrets(&self, key_dir: Option<&Path>) -> Result { + /// Open every value for a Build: each whole by name, with the parts that hide + /// them in what it reports. + fn open_for_build(&self, key_dir: Option<&Path>) -> Result; +} + +impl Unseal for SealedRecord { + fn secrets(&self, key_dir: Option<&Path>) -> Result { let key = load(key_dir)?; - Ok(self - .opened(&key)? + Ok(opened(self, &key)? .into_iter() .flat_map(|opened| { std::iter::once((opened.name.to_owned(), opened.whole)).chain(opened.parts) @@ -114,87 +110,77 @@ impl SealedRecord { .collect()) } - /// Open every value for a Build on this Machine: the values by name for - /// its secret files, and the forms that hide them in what it reports. - pub(crate) fn open_for_build( - &self, - key_dir: Option<&Path>, - ) -> Result<(BTreeMap, SealedForms), OpenError> { - self.check_bindings()?; + fn open_for_build(&self, key_dir: Option<&Path>) -> Result { + check_bindings(self)?; let key = load(key_dir)?; - let opened = self.opened(&key)?; - let secrets: SealedValues = opened - .iter() - .flat_map(|opened| { - std::iter::once((opened.name.to_owned(), opened.whole.clone())) - .chain(opened.parts.iter().cloned()) - }) - .collect(); - let values = opened - .into_iter() - .map(|opened| (opened.name.to_owned(), opened.whole)) - .collect(); - Ok((values, SealedForms::compile(&secrets))) + let mut values = BTreeMap::new(); + let mut parts = Vec::new(); + for opened in opened(self, &key)? { + values.insert(opened.name.to_owned(), opened.whole); + parts.extend(opened.parts); + } + Ok(OpenedSealed::new(values, parts)) } +} - fn opened(&self, key: &SealingKey) -> Result>, OpenError> { - self.values - .iter() - .map(|(name, value)| { - let mut opened = Opened { - name, - whole: String::new(), - parts: Vec::new(), - }; - for part in parts(value) { - match part { - Part::Text(text) => opened.whole.push_str(text), - Part::Sealed(sealed) => { - let plaintext = self.open(key, name, sealed)?; - if scrub::has_short_row(&plaintext) { - return Err(OpenError::ShortLine(name.clone())); - } - opened.whole.push_str(&plaintext); - opened.parts.push((sealed.key.clone(), plaintext)); +fn opened<'a>(record: &'a SealedRecord, key: &SealingKey) -> Result>, OpenError> { + record + .values() + .iter() + .map(|(name, value)| { + let mut opened = Opened { + name, + whole: String::new(), + parts: Vec::new(), + }; + for part in parts(value) { + match part { + Part::Text(text) => opened.whole.push_str(text), + Part::Sealed(sealed) => { + let plaintext = open_value(record, key, name, sealed)?; + if scrub::has_short_row(&plaintext) { + return Err(OpenError::ShortLine(name.clone())); } + opened.whole.push_str(&plaintext); + opened.parts.push((sealed.key.clone(), plaintext)); } } - Ok(opened) - }) - .collect() - } + } + Ok(opened) + }) + .collect() +} - fn check_bindings(&self) -> Result<(), OpenError> { - if self.values.contains_key("PORT") { - return Err(OpenError::SealedPort); +fn check_bindings(record: &SealedRecord) -> Result<(), OpenError> { + if record.values().contains_key("PORT") { + return Err(OpenError::SealedPort); + } + for (name, value) in record.values() { + if name.is_empty() || name.contains(['=', '\0']) { + return Err(OpenError::BadName(name.clone())); } - for (name, value) in &self.values { - if name.is_empty() || name.contains(['=', '\0']) { - return Err(OpenError::BadName(name.clone())); - } - if let EnvValue::Sealed(sealed) = value { - let scope_matches = match sealed.scope { - SealedScope::Shared => true, - SealedScope::Service(service) => service == self.identity.service, - }; - if sealed.key != *name || !scope_matches { - return Err(OpenError::Unbound(name.clone())); - } + if let EnvValue::Sealed(sealed) = value { + let scope_matches = match sealed.scope { + SealedScope::Shared => true, + SealedScope::Service(service) => service == record.identity().service, + }; + if sealed.key != *name || !scope_matches { + return Err(OpenError::Unbound(name.clone())); } } - Ok(()) } + Ok(()) +} - fn open( - &self, - key: &SealingKey, - name: &str, - sealed: &SealedValue, - ) -> Result { - key.open_value(&sealed.associated_data(&self.identity), &sealed.sealed) - .and_then(|bytes| String::from_utf8(bytes.to_vec()).ok()) - .ok_or_else(|| OpenError::Unopenable(name.to_owned())) - } +fn open_value( + record: &SealedRecord, + key: &SealingKey, + name: &str, + sealed: &SealedValue, +) -> Result { + key.open_value(&sealed.associated_data(record.identity()), &sealed.sealed) + .and_then(|bytes| String::from_utf8(bytes.to_vec()).ok()) + .ok_or_else(|| OpenError::Unopenable(name.to_owned())) } pub(crate) enum Lines { @@ -494,7 +480,7 @@ mod tests { assert_eq!(format!("{secrets:?}"), r#"["PASSWORD", "TOKEN", "URL"]"#); let label = serde_json::to_string(&record).unwrap(); assert_eq!( - SealedRecord::from_label(Some(&label)).unwrap(), + record_from_label(Some(&label)).unwrap(), Some(record.clone()) ); assert!(!serde_json::to_string(&record).unwrap().contains("t0ken")); diff --git a/core/crates/ployzd/tests/management_connect.rs b/core/crates/ployzd/tests/management_connect.rs index 586b8be9ef..d2cc8934d3 100644 --- a/core/crates/ployzd/tests/management_connect.rs +++ b/core/crates/ployzd/tests/management_connect.rs @@ -1074,14 +1074,14 @@ async fn build_grant_contract() { .unwrap() .capability .unwrap(); - let minted = machine.mint(); + let (id, grant) = machine.mint(); let relay = &machine.relay; let grants = &machine.grants; let seen = &machine.seen; // The grant key is not a Management Capability: Machine RPC refuses it. let connector = SystemConnector::default().with_management_relay(relay.clone()); - let as_capability = ManagementCapability::new(*minted.grant.machine(), *minted.grant.secret()); + let as_capability = ManagementCapability::new(*grant.machine(), *grant.secret()); assert_refused(connector.connect(&connection(&as_capability)).await); // A Management Capability's key holds no grant, so it cannot push. let stranger = open_grant_registry( @@ -1098,7 +1098,7 @@ async fn build_grant_contract() { .is_err() ); - let registry = open_grant_registry(&minted.grant, relay).await.unwrap(); + let registry = open_grant_registry(&grant, relay).await.unwrap(); let base = format!("http://{}/v2/ployz-build/web", registry.address()); let blob = format!("{base}/blobs/sha256:{}", "a".repeat(64)); assert_eq!(http.head(&blob).send().await.unwrap().status(), 200); @@ -1151,7 +1151,7 @@ async fn build_grant_contract() { // Reading the grant reports the push without spending anything more, and the // spent grant refuses a redial. - let inspected = grants.inspect(&minted.id).unwrap(); + let inspected = grants.inspect(&id).unwrap(); assert_eq!( inspected .pushed @@ -1161,7 +1161,7 @@ async fn build_grant_contract() { ); assert!(inspected.expires_in_seconds > 0); assert_eq!(http.head(&blob).send().await.unwrap().status(), 403); - let again = open_grant_registry(&minted.grant, relay).await.unwrap(); + let again = open_grant_registry(&grant, relay).await.unwrap(); assert!( http.get(format!("http://{}/v2/", again.address())) .send() @@ -1169,7 +1169,56 @@ async fn build_grant_contract() { .is_err() ); assert!(again.refusal().is_some()); - assert_eq!(grants.inspect(&minted.id).unwrap().pushed, inspected.pushed); + assert_eq!(grants.inspect(&id).unwrap().pushed, inspected.pushed); + machine.stop().await; +} + +/// The runner makes the key and keeps its secret: the Machine admits that key alone. +#[tokio::test(flavor = "multi_thread")] +async fn a_holder_grant_admits_only_the_holder_key() { + tokio::time::timeout(Duration::from_secs(120), holder_contract()) + .await + .expect("build grant test timed out"); +} + +async fn holder_contract() { + use ployz_core::{BuildGrant, BuildGrantId, MintedKey}; + + let machine = GrantMachine::start().await; + let holder = iroh::SecretKey::generate(); + let id = BuildGrantId::parse(hex::encode(holder.public().as_bytes())).unwrap(); + let minted = machine.mint_for(Some(id)); + assert_eq!(minted.id, id); + let MintedKey::Holder { machine: dial } = minted.key else { + panic!("a holder mint returns the Machine to dial, never a secret"); + }; + let http = reqwest::Client::new(); + let blob = format!("/v2/ployz-build/web/blobs/sha256:{}", "a".repeat(64)); + let status = async |secret: [u8; 32], method: reqwest::Method, path: &str| { + let registry = open_grant_registry(&BuildGrant::new(dial, secret), &machine.relay) + .await + .unwrap(); + http.request(method, format!("http://{}{path}", registry.address())) + .send() + .await + .map(|response| response.status()) + .ok() + }; + let stranger = iroh::SecretKey::generate().to_bytes(); + assert_eq!(status(stranger, reqwest::Method::HEAD, &blob).await, None); + assert_eq!( + status(stranger, reqwest::Method::GET, "/ployz/sealed").await, + None + ); + assert_eq!( + status(holder.to_bytes(), reqwest::Method::HEAD, &blob).await, + Some(reqwest::StatusCode::OK) + ); + // This grant carries no Sealed values. + assert_eq!( + status(holder.to_bytes(), reqwest::Method::GET, "/ployz/sealed").await, + Some(reqwest::StatusCode::NOT_FOUND) + ); machine.stop().await; } @@ -1186,10 +1235,8 @@ async fn already_held_contract() { let machine = GrantMachine::start().await; let http = reqwest::Client::new(); let head = async |reference: String| { - let minted = machine.mint(); - let registry = open_grant_registry(&minted.grant, &machine.relay) - .await - .unwrap(); + let (id, grant) = machine.mint(); + let registry = open_grant_registry(&grant, &machine.relay).await.unwrap(); let manifests = format!("http://{}/v2/ployz-build/web/manifests", registry.address()); let status = http .head(format!("{manifests}/{reference}")) @@ -1198,7 +1245,7 @@ async fn already_held_contract() { .unwrap() .status(); assert_eq!(status, 200, "{reference}"); - (minted, manifests, registry) + (id, manifests, registry) }; // Another tag, the held digest by digest, or a tag whose digest the ingest does @@ -1208,12 +1255,12 @@ async fn already_held_contract() { format!("sha256:{HELD}"), format!("ployz-sha256-{}", "b".repeat(64)), ] { - let (minted, _, _registry) = head(reference.clone()).await; - let inspected = machine.grants.inspect(&minted.id).unwrap(); + let (id, _, _registry) = head(reference.clone()).await; + let inspected = machine.grants.inspect(&id).unwrap(); assert_eq!(inspected.pushed, None, "{reference}"); } - let (minted, manifests, _registry) = head(format!("ployz-sha256-{HELD}")).await; + let (id, manifests, _registry) = head(format!("ployz-sha256-{HELD}")).await; // One push per grant: the held image spent it. let put = http .put(format!("{manifests}/ployz-sha256-{HELD}")) @@ -1222,7 +1269,7 @@ async fn already_held_contract() { .await .unwrap(); assert_eq!(put.status(), 403); - let inspected = machine.grants.inspect(&minted.id).unwrap(); + let inspected = machine.grants.inspect(&id).unwrap(); assert_eq!( inspected .pushed @@ -1284,13 +1331,29 @@ impl GrantMachine { } } + /// A grant for one push into `ployz-build/web`, held by `holder` when given. + fn mint_for(&self, holder: Option) -> ployz_core::BuildGrantMinted { + let request = ployz_core::MintBuildGrantRequest { + repository: ployz_core::BuildGrantRepository::parse("ployz-build/web").unwrap(), + holder, + sealed: None, + }; + self.grants + .mint( + self.local.record().management_secret().public_key(), + ployzd::management::Mint::check(request, None).unwrap(), + self.ingest, + ) + .unwrap() + } + /// A grant for one push into `ployz-build/web`. - fn mint(&self) -> ployz_core::BuildGrantMinted { - self.grants.mint( - self.local.record().management_secret().public_key(), - ployz_core::BuildGrantRepository::parse("ployz-build/web").unwrap(), - self.ingest, - ) + fn mint(&self) -> (ployz_core::BuildGrantId, ployz_core::BuildGrant) { + let minted = self.mint_for(None); + let ployz_core::MintedKey::Grant { grant } = minted.key else { + panic!("a holderless mint returns its grant"); + }; + (minted.id, grant) } async fn stop(self) { diff --git a/dashboard/src/modules/config-store/config-store.server.ts b/dashboard/src/modules/config-store/config-store.server.ts index f5aa75da37..7175873ac5 100644 --- a/dashboard/src/modules/config-store/config-store.server.ts +++ b/dashboard/src/modules/config-store/config-store.server.ts @@ -50,6 +50,13 @@ export const cancelDispatchedGithubRun = (run: GithubBuild["run"]) => Effect.catch((error) => Effect.logWarning("Could not cancel a GitHub run Cloud dispatched.", { runId: run.run_id, error })), ); +/** The build GitHub holds, or held until it ended; `null` once GitHub passed it to the next Builder. */ +export const heldGithubBuild = Effect.fn("ConfigStore.heldGithubBuild")(function* (buildId: string) { + const store = yield* cloudStore; + const handOff = yield* storeTry(() => store.githubBuild(buildId)); + return "held" in handOff ? handOff.held : null; +}); + /** * Stop a Deployment's builds still on GitHub: fail them and cancel their runs. Their Build Grants expire by * themselves. Idempotent. Its cancellation, and a worker that gave up on it, call it. @@ -81,7 +88,7 @@ export const stopGithubInstallationBuilds = Effect.fn("ConfigStore.stopGithubIns Effect.catchIf(refusedWith("not_found"), () => Effect.succeed(null))); for (const build of view?.builds ?? []) { if (build.status !== "building") continue; - const github = yield* storeTry(() => store.githubBuild(`${deploymentId}.${build.service}`)).pipe( + const github = yield* heldGithubBuild(`${deploymentId}.${build.service}`).pipe( Effect.catchIf(refusedWith("not_found"), () => Effect.succeed(null))); if (github === null || github.status !== "building" || github.run.installation_id !== installationId) continue; yield* cancelDispatchedGithubRun(github.run); diff --git a/dashboard/src/modules/config-store/store-deployment.inngest.postgres.test.ts b/dashboard/src/modules/config-store/store-deployment.inngest.postgres.test.ts index c3273c7dba..7187d36a8b 100644 --- a/dashboard/src/modules/config-store/store-deployment.inngest.postgres.test.ts +++ b/dashboard/src/modules/config-store/store-deployment.inngest.postgres.test.ts @@ -233,8 +233,8 @@ it.live( expect(first.result).toMatchObject({ ran: { id: DEPLOYED, status: "failed" } }); expect(yield* view(DEPLOYED)).toMatchObject({ outcome: { type: "not_executed", reason: "No Server is enrolled in this Organization" }, - // Auto tries GitHub first; a public repository has no GitHub App to run it, so the servers take it. - builds: [{ service: "web", commit: HEAD, status: "pending", message: "GitHub builds need the Ployz GitHub App on acme/web" }], + // Auto tries GitHub first; with no Server to read, GitHub passes it on, and that first reason stays. + builds: [{ service: "web", commit: HEAD, status: "pending", message: "Cloud couldn't read your Servers: No Server is enrolled in this Organization" }], }); expect(state).toMatchObject({ heads: 1, archives: [HEAD] }); diff --git a/dashboard/src/modules/config-store/store-github-builds.postgres.test.ts b/dashboard/src/modules/config-store/store-github-builds.postgres.test.ts index 9c395d458e..e95fa4e737 100644 --- a/dashboard/src/modules/config-store/store-github-builds.postgres.test.ts +++ b/dashboard/src/modules/config-store/store-github-builds.postgres.test.ts @@ -9,6 +9,7 @@ import { callStore, gatherTrusted } from "#/modules/config-store/config-store.se import { permissionsOf } from "#/modules/organization/permissions"; import { checkInStoreGithubBuild, checkStoreGithubBuild, planStoreGithubBuilds, recordStoreGithubBuildSteps, startStoreGithubBuild, + storeGithubBuildOpen, } from "#/modules/config-store/store-github-builds.server"; import { cloudStore } from "#/modules/config-store/store-sdk.server"; import { GithubOidcKeys } from "#/modules/github/github-oidc.server"; @@ -93,7 +94,7 @@ it.live( })); const request = (token: string) => new Request(`http://localhost:3000/api/builds/${next}/check-in`, { method: "POST", headers: { authorization: `Bearer ${token}` } }); - const rejection = (token: string) => provided(Effect.flip(checkInStoreGithubBuild(request(token), next))); + const rejection = (token: string) => provided(Effect.flip(checkInStoreGithubBuild(request(token), next, ""))); expect(yield* rejection(oidcToken({}, crypto.generateKeyPairSync("rsa", { modulusLength: 2048 }).privateKey))).toMatchObject({ _tag: "Unauthorized" }); expect(yield* rejection(oidcToken({ repository_id: "43" }))).toMatchObject({ _tag: "Forbidden", message: "The token is for another repository" }); expect(yield* rejection(oidcToken({ job_workflow_ref: WORKFLOW.replace("main", "evil") }))) @@ -102,6 +103,15 @@ it.live( expect(yield* rejection(oidcToken({ event_name: "push" }))).toMatchObject({ _tag: "Forbidden", message: "The run was not dispatched by Ployz" }); // The right run with no Server to mint a grant: the runner retries its check-in. expect(yield* rejection(oidcToken())).toMatchObject({ _tag: "BuildGrantUnavailable" }); + const holding = (body: string) => provided(Effect.flip(checkInStoreGithubBuild(new Request( + `http://localhost:3000/api/builds/${next}/check-in`, + { method: "POST", headers: { authorization: `Bearer ${oidcToken()}`, "content-type": "application/json" } }, + ), next, body))); + expect(yield* holding(JSON.stringify({ holder: "d".repeat(64) }))).toMatchObject({ _tag: "BuildGrantUnavailable" }); + expect(yield* holding(JSON.stringify({}))).toMatchObject({ _tag: "BuildGrantUnavailable" }); + for (const body of [JSON.stringify({ holder: "D".repeat(64) }), JSON.stringify({ holder: "d".repeat(63) }), "[]", "not json"]) { + expect(yield* holding(body)).toMatchObject({ _tag: "Validation" }); + } // Build Steps before the check-in are refused. const steps = yield* provided(Effect.flip(recordStoreGithubBuildSteps(request(oidcToken()), next, JSON.stringify({ from: 0, events: [] })))); expect(steps).toMatchObject({ _tag: "Conflict", message: "This build has not checked in" }); @@ -111,7 +121,7 @@ it.live( operation: "write", command: { command: "cancel", deployment: "00000000-0000-4000-8000-00000000b102" }, }, { source: "dashboard" }, { approval: "not_required", permissions: permissionsOf("owner") })); expect(cancelled).toMatchObject({ ok: true }); - expect(yield* Effect.promise(() => store.githubBuild(next))).toMatchObject({ status: "failed" }); + expect(yield* Effect.promise(() => store.githubBuild(next))).toMatchObject({ held: { status: "failed" } }); expect(yield* rejection(oidcToken())).toMatchObject({ _tag: "Conflict" }); // A walk that starts with the Servers leaves the build to them, unless no Server takes builds: then GitHub. @@ -167,14 +177,14 @@ it.live( run_id: 9105, run_url: "https://github.com/acme/web/actions/runs/9105", workflow_ref: WORKFLOW, repository: "acme/web", installation_id: 7, })); expect(yield* provided(unlinkGithubInstallation({ organizationId: ORGANIZATION, installationId: 7 }))).toBe(true); - expect(yield* provided(Effect.flip(checkInStoreGithubBuild(request(oidcToken({ run_id: "9105" })), held)))) + expect(yield* provided(Effect.flip(checkInStoreGithubBuild(request(oidcToken({ run_id: "9105" })), held, "")))) .toMatchObject({ _tag: "Forbidden", message: "GitHub account `acme` was disconnected" }); expect(yield* provided(Effect.flip(recordStoreGithubBuildSteps(request(oidcToken({ run_id: "9105" })), held, JSON.stringify({ from: 0, events: [] }))))) .toMatchObject({ _tag: "Forbidden", message: "GitHub account `acme` was disconnected" }); - expect(yield* Effect.promise(() => store.githubBuild(held))).toMatchObject({ status: "building" }); + expect(yield* Effect.promise(() => store.githubBuild(held))).toMatchObject({ held: { status: "building" } }); const look = yield* provided(checkStoreGithubBuild(ORGANIZATION, { ...target, build: held }, { ended: false, startLimit: false })); expect(look).toEqual({ failed: "GitHub account `acme` was disconnected" }); - expect(yield* Effect.promise(() => store.githubBuild(held))).toMatchObject({ status: "failed" }); + expect(yield* Effect.promise(() => store.githubBuild(held))).toMatchObject({ held: { status: "failed" } }); }), 60_000, ); @@ -207,3 +217,48 @@ it.live("a build that checked in reports its steps until its Organization discon expect(yield* provided(Effect.flip(steps()))).toMatchObject({ _tag: "Forbidden", message: "GitHub account `acme` was disconnected" }); expect(githubReport).toHaveBeenCalledTimes(1); }), 60_000); + +it.live("a build GitHub passed on before its run checked in is no longer GitHub's, even once the next Builder claims it", () => + Effect.gen(function* () { + const services = yield* storeTestCloud({ github: github.service }); + const provided = (program: Effect.Effect) => program.pipe( + Effect.provide(services), + Effect.provideService(GithubOidcKeys, { keys: Effect.succeed([jwk]) }), + ); + yield* provided(seedStoreOrganization(ORGANIZATION)); + const store = yield* provided(seedStoreGitService(ORGANIZATION, { project: PROJECT, environment: ENVIRONMENT, service: SERVICE })); + yield* Effect.promise(() => store.write(ORGANIZATION, { + command: "admit", admit: "deploy", id: DEPLOYMENT, environment: here, services: [], version: null, accept_volume_loss: [], + }, acmeWeb())); + yield* Effect.promise(() => store.pinSources(DEPLOYMENT, { web: HEAD })); + yield* Effect.promise(() => store.githubDispatched(BUILD, { + run_id: RUN, run_url: `https://github.com/acme/web/actions/runs/${RUN}`, workflow_ref: WORKFLOW, repository: "acme/web", installation_id: 7, + })); + // The start limit passes it on before any grant, the same Skip a check-in records when only the next Builder can + // build a sealed value. That one needs a Machine to answer the mint, which no test here runs. + expect(yield* Effect.promise(() => store.githubFinish(BUILD, true, []))).toEqual({ ended: "pending" }); + expect(yield* Effect.promise(() => store.githubBuild(BUILD))).toEqual({ passed_on: { why: "no runner started the build in time" } }); + + const target = { + build: BUILD, service: "web", repository: "acme/web", repositoryId: 42, installationId: 7, hasNext: true, + }; + const passedOn = { _tag: "Conflict", message: "GitHub passed this build to the next Builder." }; + const runner = () => new Request(`http://localhost:3000/api/builds/${BUILD}/check-in`, { + method: "POST", headers: { authorization: `Bearer ${oidcToken()}` }, + }); + const late = Effect.gen(function* () { + expect(yield* provided(Effect.flip(checkInStoreGithubBuild(runner(), BUILD, "")))).toMatchObject(passedOn); + expect(yield* provided(Effect.flip(recordStoreGithubBuildSteps(runner(), BUILD, JSON.stringify({ from: 0, events: [] }))))) + .toMatchObject(passedOn); + expect(yield* provided(storeGithubBuildOpen(target))).toBe(false); + expect(yield* provided(checkStoreGithubBuild(ORGANIZATION, target, { ended: true, startLimit: false }))).toBe("done"); + }); + yield* late; + + // The next Builder takes it, with why; the run GitHub passed it on from still holds nothing. + expect(yield* Effect.promise(() => store.claimDeployment(ORGANIZATION, DEPLOYMENT, "cloud-1"))).toMatchObject({ id: DEPLOYMENT }); + expect(yield* Effect.promise(() => store.read(ORGANIZATION, { query: "deployment", id: DEPLOYMENT }))).toMatchObject({ + builds: [{ service: "web", status: "pending", message: "no runner started the build in time" }], + }); + yield* late; + }), 60_000); diff --git a/dashboard/src/modules/config-store/store-github-builds.server.ts b/dashboard/src/modules/config-store/store-github-builds.server.ts index d1590cd8aa..70153664a9 100644 --- a/dashboard/src/modules/config-store/store-github-builds.server.ts +++ b/dashboard/src/modules/config-store/store-github-builds.server.ts @@ -1,7 +1,7 @@ import "@tanstack/react-start/server-only"; import type { GithubBuild, GithubClaims } from "@ployz/sdk"; -import { Effect } from "effect"; -import { cancelDispatchedGithubRun, cancelStoreGithubBuilds, connectionsOf } from "#/modules/config-store/config-store.server"; +import { Effect, Schema } from "effect"; +import { cancelDispatchedGithubRun, cancelStoreGithubBuilds, connectionsOf, heldGithubBuild } from "#/modules/config-store/config-store.server"; import { cloudStore, refusedWith, storeTry } from "#/modules/config-store/store-sdk.server"; import type { StoreRefused } from "#/modules/config-store/store.contract"; import { pinStoreSources } from "#/modules/config-store/store-deployment.server"; @@ -181,8 +181,8 @@ export const checkStoreGithubBuild = Effect.fn("StoreGithub.check")(function* ( organizationId: string, target: StoreGithubTarget, seen: { ended: boolean; startLimit: boolean }, ) { const store = yield* cloudStore; - const build = yield* storeTry(() => store.githubBuild(target.build)); - if (build.status !== "building") return "done" as const; + const build = yield* heldGithubBuild(target.build); + if (build === null || build.status !== "building") return "done" as const; const deploymentId = build.id.slice(0, build.id.indexOf(".")); const view = yield* storeTry(() => store.read(organizationId, { query: "deployment", id: deploymentId })); if (view.status !== "queued" && view.status !== "running") { @@ -216,8 +216,7 @@ export const checkStoreGithubBuild = Effect.fn("StoreGithub.check")(function* ( /** Whether a build GitHub holds is still on GitHub: a final report may settle it before a wait begins. */ export const storeGithubBuildOpen = Effect.fn("StoreGithub.open")(function* (target: StoreGithubTarget) { - const store = yield* cloudStore; - return (yield* storeTry(() => store.githubBuild(target.build))).status === "building"; + return (yield* heldGithubBuild(target.build))?.status === "building"; }); const bearer = (request: Request) => /^Bearer (\S+)$/.exec(request.headers.get("authorization") ?? "")?.[1] ?? null; @@ -243,19 +242,37 @@ const refused = (error: StoreRefused) => { } }; +const passedOn = () => new Conflict({ message: "GitHub passed this build to the next Builder." }); + +const MAX_CHECK_IN_BYTES = 4 * 1024; +const CheckIn = Schema.fromJsonString(Schema.Struct({ + holder: Schema.optional(Schema.NullOr(Schema.String.check(Schema.isPattern(/^[0-9a-f]{64}$/)))), +})); +const notCheckIn = () => new Validation({ message: "The check-in is not a holder key." }); + +/** The public key the runner holds its Build Grant with: none from an Action that sends no body. */ +const holderOf = Effect.fn("StoreGithub.holder")(function* (text: string) { + if (text.length > MAX_CHECK_IN_BYTES) return yield* notCheckIn(); + if (text.trim() === "") return null; + const { holder } = yield* Schema.decodeUnknownEffect(CheckIn)(text).pipe(Effect.mapError(notCheckIn)); + return holder ?? null; +}); + /** - * A runner's one check-in for a Store build: its Build Grant, pinned commit, expected fingerprint, the ployz version - * that computed it, and the one-Service deployment whose variables carry the build secrets. + * A runner's one check-in for a Store build: how it pushes, its pinned commit, expected fingerprint, the ployz + * version that computed it, and the one-Service deployment whose variables carry the build secrets. */ -export const checkInStoreGithubBuild = Effect.fn("StoreGithub.checkIn")(function* (request: Request, buildId: string) { +export const checkInStoreGithubBuild = Effect.fn("StoreGithub.checkIn")(function* (request: Request, buildId: string, text: string) { const claims = yield* runnerClaims(request); + const holder = yield* holderOf(text); const store = yield* cloudStore; return yield* Effect.gen(function* () { - const build = yield* storeTry(() => store.githubBuild(buildId)); + const build = yield* heldGithubBuild(buildId); + if (build === null) return yield* passedOn(); const refusal = yield* installationRefusal(build.organization, build.run.installation_id); if (refusal !== null) return yield* new Forbidden({ message: refusal.message }); const connections = yield* connectionsOf(build.organization); - return yield* storeTry(() => store.githubCheckIn(buildId, claims, connections)); + return yield* storeTry(() => store.githubCheckIn(buildId, claims, holder, connections)); }).pipe(Effect.catchTag("StoreRefused", (error) => Effect.fail(refused(error)))); }); @@ -271,7 +288,8 @@ export const recordStoreGithubBuildSteps = Effect.fn("StoreGithub.steps")(functi const report: unknown = yield* Effect.try({ try: () => JSON.parse(text), catch: () => new Validation({ message: "The report is not Build Steps." }) }); const store = yield* cloudStore; return yield* Effect.gen(function* () { - const build = yield* storeTry(() => store.githubBuild(buildId)); + const build = yield* heldGithubBuild(buildId); + if (build === null) return yield* passedOn(); const refusal = yield* installationRefusal(build.organization, build.run.installation_id); if (refusal !== null) return yield* new Forbidden({ message: refusal.message }); const reported = yield* storeTry(() => store.githubReport(buildId, claims, report)); diff --git a/dashboard/src/modules/config-store/store-github-stop.postgres.test.ts b/dashboard/src/modules/config-store/store-github-stop.postgres.test.ts index af6747d372..12142f0699 100644 --- a/dashboard/src/modules/config-store/store-github-stop.postgres.test.ts +++ b/dashboard/src/modules/config-store/store-github-stop.postgres.test.ts @@ -78,7 +78,7 @@ for (const { stop, failed } of cases) { const target = { build: BUILD, service: "web", repository: "acme/web", repositoryId: 42, installationId: 7, hasNext: true }; expect(yield* provided(checkStoreGithubBuild(ORGANIZATION, target, { ended: true, startLimit: false }))).toEqual({ failed }); - expect(yield* Effect.promise(() => store.githubBuild(BUILD))).toMatchObject({ status: "failed" }); + expect(yield* Effect.promise(() => store.githubBuild(BUILD))).toMatchObject({ held: { status: "failed" } }); expect(cancels()).toEqual([ { path: CANCEL, access: { type: "installation", installationId: 7 } }, { path: CANCEL, access: { type: "installation", installationId: 7 } }, diff --git a/dashboard/src/modules/config-store/store-variables.test.ts b/dashboard/src/modules/config-store/store-variables.test.ts index 547c1a63e9..015fbd2c40 100644 --- a/dashboard/src/modules/config-store/store-variables.test.ts +++ b/dashboard/src/modules/config-store/store-variables.test.ts @@ -3,7 +3,6 @@ import { asTestDouble } from "#/lib/test-double"; import type { ClientSealedValue, EnvironmentRef, EnvironmentView, RowId, ServiceListing } from "@ployz/sdk"; import { serviceSettingRows } from "./store-services"; import { - firstBrowserSealed, notSetVariables, receivingService, receivingValues, @@ -202,18 +201,3 @@ it("seals a Sync row for the Service its lineage lands in, keyed by the row's va expect(await receivingValues({ sealer: async () => null }, typed, receiver, "refuse")).toEqual(typed); }); -it("names a Service's first variable the browser sealed, never a secret sealed another way", () => { - const rows = serviceSettingRows({ - ...view, - settings: [ - { path: "web.env.API_KEY", value: { secret: true }, default: null, apply: "staged", row: "w:variables.API_KEY" as RowId }, - { path: "web.env.NPM_TOKEN", value: { secret: true }, default: null, apply: "staged", row: "w:variables.NPM_TOKEN" as RowId }, - { path: "web.env.AUTH", value: { secret: true }, default: null, apply: "staged", row: "w:variables.AUTH" as RowId }, - { path: "api.env.AAA", value: { secret: true }, default: null, apply: "staged", row: "a:variables.AAA" as RowId }, - ], - }, "web"); - const sealed = (...marked: string[]) => firstBrowserSealed(rows, marked.map((row) => row as RowId)); - expect(sealed()).toBeNull(); - expect(sealed("a:variables.AAA")).toBeNull(); - expect(sealed("w:variables.NPM_TOKEN", "w:variables.AUTH")).toBe("AUTH"); -}); diff --git a/dashboard/src/modules/config-store/store-variables.ts b/dashboard/src/modules/config-store/store-variables.ts index a0d8e30223..7f6ef3b492 100644 --- a/dashboard/src/modules/config-store/store-variables.ts +++ b/dashboard/src/modules/config-store/store-variables.ts @@ -50,16 +50,6 @@ type Write = Change | { readonly key: string; readonly sealed: string }; const UNPLACED = "Set this value after merging."; -/** The first of a Service's variables, by key, whose value the browser sealed (`EnvironmentView.sealed`). */ -export function firstBrowserSealed(rows: ReadonlyMap, sealed: readonly RowId[] = []) { - const marked = new Set(sealed); - const keys = [...rows].flatMap(([name, row]) => { - const key = VARIABLE.exec(name)?.[1]; - return key !== undefined && row.row !== undefined && marked.has(row.row) ? [key] : []; - }); - return keys.sort()[0] ?? null; -} - const leavesValueless = Schema.is(Schema.Struct({ after: Schema.Struct({ secret: Schema.Literal(false) }) })); /** diff --git a/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreGitFields.tsx b/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreGitFields.tsx index 85505c9317..70ffd00360 100644 --- a/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreGitFields.tsx +++ b/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreGitFields.tsx @@ -104,17 +104,15 @@ export function StoreDockerfileField({ organizationSlug, gitRef, branch, value, } /** Who builds a Git Service first; part of its Deployment Policy, so it applies at once and Discard leaves it. */ -export function StorePreferredBuilderField({ organizationSlug, value, sealed, onSet }: { +export function StorePreferredBuilderField({ organizationSlug, value, onSet }: { organizationSlug: string; value: string | null; - /** The first of the Service's variables the browser sealed: GitHub never builds it. */ - sealed: string | null; onSet: (builder: string | null) => PersistableTransaction; }) { const setting = serviceSetting("preferredBuilder"); const { machines, status } = useRuntimeLens(organizationSlug); const builders = [ - { id: "github", label: "GitHub Actions", refusal: sealed === null ? null : `Uses sealed ${sealed}` }, - ...machines.filter((machine) => machine.acceptsBuilds).map((machine) => ({ id: machine.id, label: machine.name, refusal: null })), + { id: "github", label: "GitHub Actions" }, + ...machines.filter((machine) => machine.acceptsBuilds).map((machine) => ({ id: machine.id, label: machine.name })), ]; const [current, hold] = useCommitHold(value ?? "auto"); const label = current === "auto" ? "Auto" @@ -130,12 +128,7 @@ export function StorePreferredBuilderField({ organizationSlug, value, sealed, on {builders.map((builder) => ( - - - {builder.label} - {builder.refusal === null ? null : {builder.refusal}} - - + {builder.label} ))} diff --git a/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreServiceDrawer.tsx b/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreServiceDrawer.tsx index cd667eaa7f..c996050e22 100644 --- a/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreServiceDrawer.tsx +++ b/dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/services/$serviceId/-components/StoreServiceDrawer.tsx @@ -25,7 +25,6 @@ import { changedProps, deployedVcpuLimit, serviceChanges, serviceSettingRows, se import { diffQuery, environmentSettingsQuery, namespaceQuery, requireView, servicesQuery, useStoreViews, volumesQuery } from "#/modules/config-store/store-view.queries"; import type { Persistable } from "#/collections/query-collection"; import { useStoreWriter } from "#/modules/config-store/store-write"; -import { firstBrowserSealed } from "#/modules/config-store/store-variables"; import { useCommitHold } from "#/components/stageable/use-commit-hold"; import { useStorePendingName, useStorePendingNodes } from "#/modules/config-store/store-pending"; import { SettingsAddButton, SettingsDanger, SettingsFoot, SettingsRow, SettingsSection } from "#/components/settings/settings-section"; @@ -193,7 +192,6 @@ export function StoreServiceDrawer({ params }: { params: { organizationSlug: str {field("buildMethod")} {buildMethod === "dockerfile" ? : field("buildCommand")} state.set("preferredBuilder", builder)} /> : null, deploy: <> diff --git a/dashboard/src/routes/api/builds/$build/check-in.ts b/dashboard/src/routes/api/builds/$build/check-in.ts index 541c789ebd..09a63f4ed0 100644 --- a/dashboard/src/routes/api/builds/$build/check-in.ts +++ b/dashboard/src/routes/api/builds/$build/check-in.ts @@ -9,9 +9,10 @@ export const Route = createFileRoute("/api/builds/$build/check-in")({ server: { handlers: { POST: async ({ request, params }) => { + const text = await request.text(); const options = { signal: request.signal }; // A build's id is DEPLOYMENT.SERVICE. - return runAppEffect(checkInStoreGithubBuild(request, params.build).pipe(Effect.map((body) => Response.json(body))), options) + return runAppEffect(checkInStoreGithubBuild(request, params.build, text).pipe(Effect.map((body) => Response.json(body))), options) .catch(publicErrorResponse); }, ANY: (): Response => methodNotAllowed(Route), diff --git a/docs/user/builds/overview.md b/docs/user/builds/overview.md index a7f124828e..cb5183208b 100644 --- a/docs/user/builds/overview.md +++ b/docs/user/builds/overview.md @@ -68,8 +68,9 @@ server more time, see [Build resources and cache](build-resources-and-cache.md#g ran. Changing replicas, domains, or CPU and memory limits doesn't. - **Git submodules, Git LFS and very large repositories don't build.** Keep large files out of the repository; see [Troubleshooting builds](../troubleshooting/builds.md#my-repository-is-too-big-or-uses-submodules). -- **A build gets your service's variables, secrets included.** See - [Use variables at build time](railpack-and-dockerfiles.md#use-variables-at-build-time). +- **A build gets your service's variables, sealed values included,** on GitHub Actions too. + See [Use variables at build time](railpack-and-dockerfiles.md#use-variables-at-build-time) + and [Build on GitHub Actions](where-builds-run.md#build-on-github-actions). Next: [change how your app builds](railpack-and-dockerfiles.md), with commands or your own Dockerfile. diff --git a/docs/user/builds/railpack-and-dockerfiles.md b/docs/user/builds/railpack-and-dockerfiles.md index abfb21c70b..6ff8900569 100644 --- a/docs/user/builds/railpack-and-dockerfiles.md +++ b/docs/user/builds/railpack-and-dockerfiles.md @@ -77,8 +77,11 @@ Builds on [GitHub Actions](where-builds-run.md#build-on-github-actions) get your ### Read a sealed value -A build that reads a sealed value [runs on your server](where-builds-run.md). -Railpack builds see it as they see any other variable. A Dockerfile reads it with `ARG`: +Railpack builds see a sealed value as they see any other variable, on your servers and on +[GitHub Actions](where-builds-run.md#build-on-github-actions). A sealed `RAILPACK_*` setting +builds only on GitHub Actions: your servers refuse it. A **Build command** replaces a sealed +`RAILPACK_BUILD_CMD`, so that build runs on your servers too. A Dockerfile reads a sealed value with +`ARG`: ```dockerfile ARG NPM_TOKEN @@ -91,6 +94,9 @@ A secret mount keeps the value out of the image's history: RUN --mount=type=secret,id=NPM_TOKEN,env=NPM_TOKEN npm ci ``` +On GitHub Actions, a build that reads a sealed value with `ARG`, or sets a `RAILPACK_*` +variable to a sealed value, saves no build cache. A secret mount keeps it. + A changed sealed value doesn't rerun a cached secret mount step. To rerun it, add `ARG PLOYZ_SEALED_HASH` right above it. diff --git a/docs/user/builds/where-builds-run.md b/docs/user/builds/where-builds-run.md index 84aaef7da4..293a71534e 100644 --- a/docs/user/builds/where-builds-run.md +++ b/docs/user/builds/where-builds-run.md @@ -32,9 +32,11 @@ secrets. The **GitHub Actions** section appears once a service builds from a rep through the Ployz GitHub App. > [!NOTE] -> A GitHub build receives your service's variables, secrets included, so it can build your -> app the same way your servers would. To keep them on your servers, choose **Your servers -> only** and don't set any service's **Preferred Builder** to **GitHub Actions**. +> A GitHub build receives your service's variables, sealed values included, so it can build +> your app the same way your servers would. Your server unlocks sealed values only for that +> build's runner, and Cloud passes them on still sealed. The build log hides them. To keep +> your variables off GitHub, choose **Your servers only** and don't set any service's +> **Preferred Builder** to **GitHub Actions**. 1. Go to **Organization → Builds**. 2. Under **GitHub Actions**, click **Add workflow ↗** next to the repository. GitHub opens the @@ -119,9 +121,6 @@ goes to GitHub only when none of them does. If the preferred server is gone or d builds, another of your servers takes the build. **Auto** follows the build order alone. **Preferred Builder** applies at once, from the next build. -Builds that read a sealed value run on your server. GitHub Actions is skipped for them, -whatever the service prefers. - A deploy names the server each service built on. When GitHub came first and couldn't take the build, for example because the repository has no Ployz build workflow, the deploy says so and why. @@ -141,7 +140,5 @@ When no builder can take a build, it fails with one of these: - `…No other Builder in your Build Order can take it`. GitHub was last and couldn't take it. The build log says why GitHub was skipped. Fix that, or pick an order that ends with your servers. -- `…Builds that read a sealed value run on your server. No other Builder in your Build Order can - take it`. The order is **GitHub only**. Pick one with your servers. See [Troubleshooting builds](../troubleshooting/builds.md) for failed build steps.