Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions crates/engine/src/engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ pub struct Engine {
/// memory and never touch the SQLite WAL; entries over the per-entry cap
/// spill to `local_cache`. See [`LocalCacheTmp`].
pub(crate) local_cache_tmp: Arc<dyn LocalCache>,
/// Mem-only store for `secret` targets. Unlike [`local_cache_tmp`], it never
/// spills to disk — secret outputs must be held in memory only. Backed by a
/// deny-durable, so an unexpected spill errors loudly instead of writing the
/// secret to disk.
///
/// [`local_cache_tmp`]: Self::local_cache_tmp
pub(crate) local_cache_secret: Arc<dyn LocalCache>,
/// Shared cross-run filesystem-walk cache (separate `fswalk.db`), handed to
/// tree-walking plugins via [`PluginInit`].
pub(crate) walker: Arc<hwalk::CachedWalker>,
Expand Down Expand Up @@ -348,6 +355,16 @@ impl Engine {
cfg.tmp_cache.capacity_bytes,
));

// Secret targets: a mem-only store with effectively unbounded limits so
// entries never spill, backed by a deny-durable so any spill (which must
// not happen) errors instead of writing a secret to disk.
let local_cache_secret: Arc<dyn LocalCache> =
Arc::new(crate::engine::local_cache_tmp::LocalCacheTmp::new(
Arc::new(crate::engine::local_cache::DenyDurable),
usize::MAX,
u64::MAX,
));

// Best-effort sweep of stale `sandboxfuse<pid>` dirs from crashed runs.
sweep_stale_sandboxfuse_dirs(&home);

Expand Down Expand Up @@ -376,6 +393,7 @@ impl Engine {
home: home.clone(),
local_cache,
local_cache_tmp,
local_cache_secret,
walker,
providers: vec![],
providers_by_name: HashMap::new(),
Expand Down
95 changes: 89 additions & 6 deletions crates/engine/src/engine/local_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,31 @@ pub trait LocalCache: Send + Sync {
#[error("not found")]
pub struct NotFoundError;

/// A [`LocalCache`] backend that refuses every operation. Used as the durable
/// fallthrough for the secret cache: secret entries are held in memory and must
/// never spill to disk, so any attempt to read or write through to durable
/// storage is a bug we surface loudly rather than silently persisting a secret.
pub(crate) struct DenyDurable;

impl LocalCache for DenyDurable {
fn reader(&self, addr: &Addr, _hashin: &str, name: &str) -> anyhow::Result<SizedReader> {
anyhow::bail!(
"secret cache: refusing durable read for {addr} {name} (secret never on disk)"
)
}
fn writer(&self, addr: &Addr, _hashin: &str, name: &str) -> anyhow::Result<Box<dyn io::Write>> {
anyhow::bail!(
"secret cache: refusing durable write for {addr} {name} (secret never on disk)"
)
}
fn exists(&self, _addr: &Addr, _hashin: &str, _name: &str) -> anyhow::Result<bool> {
Ok(false)
}
fn delete(&self, _addr: &Addr, _hashin: &str, _name: &str) -> anyhow::Result<()> {
Ok(())
}
}

pub(crate) const MANIFEST_V1: &str = "manifest-v1.borsh";

#[derive(Clone)]
Expand Down Expand Up @@ -328,6 +353,7 @@ impl Engine {
hashin: &str,
artifacts: Vec<outputartifact::OutputArtifact>,
tmp: bool,
secret: bool,
) -> anyhow::Result<Vec<CacheArtifact>> {
let mut res_artifacts = Vec::with_capacity(artifacts.len());
let mut manifest_artifacts = Vec::with_capacity(artifacts.len());
Expand All @@ -342,12 +368,16 @@ impl Engine {
hashin.to_string()
};

// `tmp` (uncacheable/shell) revisions get a unique `{hashin}_{nanos}` key
// and are never read back across runs, so route them to the mem-only
// `local_cache_tmp` — small entries stay in memory and skip the SQLite
// WAL write. `CacheArtifact` carries the cache it was written to, so
// reads resolve against the same store.
let cache = if tmp {
// Secret outputs go to the mem-only `local_cache_secret`, which never
// spills to disk. A secret target is always uncached, so `tmp` is set
// and it gets the unique `{hashin}_{nanos}` key above. Other `tmp`
// (uncacheable/shell) revisions are never read back across runs, so
// route them to the mem-only `local_cache_tmp` — small entries stay in
// memory and skip the SQLite WAL write. `CacheArtifact` carries the cache
// it was written to, so reads resolve against the same store.
let cache = if secret {
&self.local_cache_secret
} else if tmp {
&self.local_cache_tmp
} else {
&self.local_cache
Expand Down Expand Up @@ -601,6 +631,56 @@ mod tests {
(engine, dir)
}

/// A secret target's outputs go to the in-memory secret store and are never
/// written to the durable local cache (the on-disk tier).
#[tokio::test]
async fn secret_outputs_stay_in_memory_and_never_touch_durable() {
let (engine, _dir) = test_engine();
let ctoken = StdCancellationToken::new();
let addr = test_addr();

let arts = engine
.cache_locally(
&ctoken,
&addr,
"HASHSECRET",
vec![raw_artifact("a", b"top secret")],
false, // tmp
true, // secret
)
.await
.expect("cache_locally");
assert_eq!(arts.len(), 1);
let art = &arts[0];

// The artifact is backed by the secret store, not the durable cache.
assert!(
Arc::ptr_eq(&art.cache, &engine.local_cache_secret),
"secret artifact must be stored in the secret cache"
);

// The durable on-disk cache must not hold the secret under any key.
assert!(
!engine
.local_cache
.exists(&addr, &art.hashin, &art.name)
.expect("exists"),
"secret output must never reach the durable cache"
);

// Still readable from the in-memory secret store.
let bytes = drain_reader(
art.cache
.reader(&addr, &art.hashin, &art.name)
.expect("secret reader")
.reader,
);
assert!(
!bytes.is_empty(),
"secret artifact must be readable in memory"
);
}

/// Engine wired to the remote cache at `remote_uri` (a `file://` dir shared
/// across engines). Returns an `Arc` so the `self: &Arc<Self>` upload helper
/// can be called.
Expand Down Expand Up @@ -645,6 +725,7 @@ mod tests {
"HASHIN1",
vec![raw_artifact("a", b"shared payload")],
false,
false,
)
.await
.expect("cache_locally");
Expand Down Expand Up @@ -707,6 +788,7 @@ mod tests {
"HASHBG",
vec![raw_artifact("a", b"bg payload")],
false,
false,
)
.await
.expect("cache_locally");
Expand Down Expand Up @@ -810,6 +892,7 @@ mod tests {
"PRIMARYHASH",
vec![raw_artifact("a", b"hello fixpoint")],
false,
false,
)
.await
.expect("cache_locally");
Expand Down
49 changes: 49 additions & 0 deletions crates/engine/src/engine/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,10 @@ impl Engine {
let hashin = opts.hashin.clone();
let spec = opts.spec.clone();
let def = opts.def.clone();
// A secret target is uncached (enforced at parse: secret requires
// `cache = False`), so `use_tmp_cache` is already true via `!enabled`.
// `secret` only selects the in-memory secret store in `cache_locally`.
let secret = opts.def.target.cache.secret;
let use_tmp_cache = !opts.def.target.cache.enabled || opts.shell;
let interactive = opts.interactive.clone();
let shell = opts.shell;
Expand Down Expand Up @@ -1764,6 +1768,7 @@ impl Engine {
&hashin,
to_cache,
use_tmp_cache,
secret,
),
)
.await
Expand Down Expand Up @@ -2075,6 +2080,8 @@ impl Engine {
def.cache.history = def.cache.history.saturating_mul(2);
}

validate_secret_cache(addr, &def.cache)?;

if def.hash.is_empty() {
anyhow::bail!("missing hash");
}
Expand Down Expand Up @@ -2321,6 +2328,20 @@ impl Engine {
/// pure functions of `def.addr` + `dest_provider`. Same dest ⇒ same stamp;
/// different dests live in distinct `mem_def` cells already keyed by addr.
/// No re-hash.
/// A secret target's outputs are held in memory only, so the target must be
/// uncached. Validated centrally here — driver-agnostic — so every driver gets
/// the same contract: the author opts out of caching explicitly (`cache =
/// False`); the engine never silently overrides a caching config.
fn validate_secret_cache(
addr: &Addr,
cache: &crate::engine::driver::targetdef::CacheConfig,
) -> anyhow::Result<()> {
if cache.secret && (cache.enabled || cache.remote_enabled) {
anyhow::bail!("target {addr} is secret and must be uncached: set `cache = False`");
}
Ok(())
}

fn rewrite_query_inputs(
mut def: crate::engine::driver::targetdef::TargetDef,
dest: &Addr,
Expand Down Expand Up @@ -4457,6 +4478,34 @@ mod tests {
Ok(())
}

#[test]
fn validate_secret_cache_requires_uncached() {
let addr = hmodel::htaddr::parse_addr("//pkg:s").expect("addr");

// Secret + local cache on → rejected, with an actionable message.
let mut c = CacheConfig::off();
c.secret = true;
c.enabled = true;
let err = validate_secret_cache(&addr, &c).expect_err("must reject cached secret");
let msg = format!("{err:#}");
assert!(msg.contains("secret"), "{msg}");
assert!(msg.contains("cache = False"), "{msg}");

// Secret + remote cache on → also rejected.
let mut c = CacheConfig::off();
c.secret = true;
c.remote_enabled = true;
assert!(validate_secret_cache(&addr, &c).is_err());

// Secret + fully uncached → ok.
let mut c = CacheConfig::off();
c.secret = true;
assert!(validate_secret_cache(&addr, &c).is_ok());

// Non-secret cached target → unaffected.
assert!(validate_secret_cache(&addr, &CacheConfig::on(true)).is_ok());
}

// ----------------------------------------------------------------------
// Per-addr result-lock self-deadlock regression (mem_locked_result).
// ----------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions crates/plugin-abi/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ fn cache_config_to_pb(c: &CacheConfig) -> pb::CacheConfig {
enabled: c.enabled,
remote_enabled: c.remote_enabled,
history: c.history,
secret: c.secret,
}
}

Expand All @@ -670,6 +671,7 @@ fn cache_config_from_pb(c: pb::CacheConfig) -> CacheConfig {
enabled: c.enabled,
remote_enabled: c.remote_enabled,
history: c.history,
secret: c.secret,
}
}

Expand Down
37 changes: 32 additions & 5 deletions crates/plugin-exec/src/pluginexec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,15 @@ impl hdriver_support::driver_managed::ManagedDriver for Driver {

let pkg = req.target_spec.addr.package.clone();

// The `secret` flag is threaded straight through; the engine validates
// that a secret target is uncached (see `validate_secret_cache`).
let cache = CacheConfig {
enabled: spec.cache.local,
remote_enabled: spec.cache.remote,
history: spec.cache.history,
secret: spec.secret,
};

// Dep groups opted into read-only staging (stage once, hardlink in)
// rather than copied into every sandbox.
let read_only_groups: std::collections::HashSet<String> =
Expand Down Expand Up @@ -639,11 +648,7 @@ impl hdriver_support::driver_managed::ManagedDriver for Driver {
.collect(),
outputs,
support_files,
cache: CacheConfig {
enabled: spec.cache.local,
remote_enabled: spec.cache.remote,
history: spec.cache.history,
},
cache,
pty: true,
hash,
transparent: false,
Expand Down Expand Up @@ -2214,6 +2219,28 @@ mod tests {
Ok(())
}

#[tokio::test]
async fn test_parse_secret_threads_flag() -> anyhow::Result<()> {
// The driver only threads `secret` into the cache config — it does not
// validate it (the engine does), so the caching values pass through.
let td = parse_with(HashMap::from([
("secret".to_string(), hcore::htvalue::Value::Bool(true)),
("cache".to_string(), hcore::htvalue::Value::Bool(false)),
]))
.await?;
assert!(td.cache.secret, "secret flag threaded");
assert!(!td.cache.enabled, "cache=False passed through");
assert!(!td.cache.remote_enabled);
Ok(())
}

#[tokio::test]
async fn test_parse_default_not_secret() -> anyhow::Result<()> {
let td = parse_with(HashMap::new()).await?;
assert!(!td.cache.secret);
Ok(())
}

#[tokio::test]
async fn test_parse_read_only_deps_annotates_only_listed_groups() -> anyhow::Result<()> {
use hcore::htvalue::Value;
Expand Down
5 changes: 5 additions & 0 deletions crates/plugin-exec/src/pluginexec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ pub(crate) struct TargetSpec {
pub codegen: CodegenMode,
/// Caching: bool toggles local+remote, or a dict `{enabled, remote, history}`.
pub cache: TargetSpecCache,
/// Mark all outputs as secret: keeps the target's outputs in memory only
/// (never written to disk). A secret target must be uncached — set
/// `cache = False` explicitly, otherwise parsing errors.
pub secret: bool,
/// Environment variables set for the command.
pub env: HashMap<String, String>,
/// Names of host environment variables passed through (hashed). `"*"` passes all.
Expand Down Expand Up @@ -148,6 +152,7 @@ mod tests {
"run",
"out",
"cache",
"secret",
"support_files",
"codegen",
"deps",
Expand Down
7 changes: 7 additions & 0 deletions crates/plugin/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,11 +482,16 @@ pub mod targetdef {
/// `enabled` gates local caching; `remote_enabled` gates the remote cache
/// (only meaningful when `enabled`). `history` is how many cache revisions
/// (distinct input hashes) to retain for this target — GC trims older ones.
///
/// `secret` marks the target's outputs as secret: they are never persisted
/// to disk (held in memory only) and the target is forced uncached. When
/// `secret` is set, `enabled`/`remote_enabled` are always false.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
pub struct CacheConfig {
pub enabled: bool,
pub remote_enabled: bool,
pub history: u32,
pub secret: bool,
}

impl CacheConfig {
Expand All @@ -499,6 +504,7 @@ pub mod targetdef {
enabled: true,
remote_enabled,
history: Self::DEFAULT_HISTORY,
secret: false,
}
}

Expand All @@ -508,6 +514,7 @@ pub mod targetdef {
enabled: false,
remote_enabled: false,
history: 0,
secret: false,
}
}
}
Expand Down
Loading
Loading