fix(stack-encrypt): an empty batch still has its context checked before any key request

A slice or Vec batch checked the call's context only through each item's
own check, so an empty batch checked nothing: a plan with no context and
none from the call, or a plan with its own context given a second one,
passed the pre-I/O check, loaded the keyset the chain named, and resolved
to an empty success. Opening a Vec of records had the same gap.

The batch impls of Runs (for fields plans and one-value plans) and Opens
(for a fields plan over Vec<FieldValues>) now check the context once,
apart from the items, before checking each item; their pending and
decryption fail on the same context, so a direct caller and a chain
agree. A context field's expected value is still checked per record,
since only a record holds the field. The one-value plans' Vec openings
already checked the context alone and only share the helper now.

Tests pin both refusals for an empty slice and Vec, run and opened,
before any keyset load, and the plan_build fuzz target checks an empty
batch's context for every plan it builds.

Author: coderdan

packages/stack-encrypt/fuzz/fuzz_targets/plan_build.rs

@@ -13,20 +13,24 @@
 //! it), and a field may be declared as the plan's context field, so a
 //! context can also be given twice.
 //!
-//! Four invariants. Building and rendering never panic, whatever the
+//! Five invariants. Building and rendering never panic, whatever the
 //! names. A plan built with its context gives every sealed or indexed field
 //! a well-formed label: `<context>/<identity>`, which renders and parses
 //! back to itself; a plan without one gives none, and keys each such field
 //! under a plain identity segment. A passthrough or context field is under
 //! no label, so a plan of passthrough fields with distinct names, under a
 //! plain context or none, builds whatever text the names are. And a plan
 //! whose context is given twice is refused with `TwoContextSources`.
+//! And a built plan checks an empty batch's call context as it would one
+//! record's: run with none and no context of its own is `NoContext`, run
+//! with one beside its own is `TwoContextSources`, and opened likewise,
+//! except that a context field's expected value waits for a record.
 
 use std::collections::HashSet;
 
 use arbitrary::Arbitrary;
 use libfuzzer_sys::fuzz_target;
-use stack_encrypt::plan::{FieldKind, FieldValues, PlanError};
+use stack_encrypt::plan::{FieldKind, FieldValues, Opens, PlanError, Runs};
 use stack_encrypt::{Equality, Error, Label, Plan};
 
 /// A name: one of a few that collide or sit on a rule's edge, or free text.
@@ -135,8 +139,10 @@ fuzz_target!(|input: Input| {
                     .unwrap_or(&declared.name)
                     .as_str();
                 assert_eq!(built.identity(), identity);
-                let keys_nothing =
-                    matches!(built.kind(), FieldKind::Passthrough | FieldKind::ContextField);
+                let keys_nothing = matches!(
+                    built.kind(),
+                    FieldKind::Passthrough | FieldKind::ContextField
+                );
                 match (built.label(), &context) {
                     (None, Some(_)) => assert!(keys_nothing),
                     (None, None) => {
@@ -160,6 +166,40 @@ fuzz_target!(|input: Input| {
                 assert!(plan.field(built.name()).is_ok());
             }
             let _ = format!("{plan:?}");
+
+            let has_own = context.is_some() || context_fields == 1;
+            let call = Label::parse("users").expect("a plain label");
+            for call in [None, Some(&call)] {
+                let expected = match (has_own, call.is_some()) {
+                    (false, false) => Some(PlanError::NoContext),
+                    (true, true) => Some(PlanError::TwoContextSources {
+                        first: if context_fields == 1 {
+                            "a context field"
+                        } else {
+                            "the plan"
+                        },
+                        second: "the call",
+                    }),
+                    _ => None,
+                };
+                let run = Runs::<Vec<FieldValues>, ()>::check(&plan, &Vec::new(), call);
+                match (&expected, run) {
+                    (None, Ok(())) => {}
+                    (Some(expected), Err(Error::Plan(error))) => assert_eq!(&error, expected),
+                    (expected, run) => panic!("an empty run checked {run:?}, not {expected:?}"),
+                }
+                let opened = Opens::<Vec<FieldValues>, ()>::check(&plan, &Vec::new(), call);
+                match (&expected, opened) {
+                    (_, Ok(())) if context_fields == 1 => {}
+                    (None, Ok(())) => {}
+                    (Some(expected), Err(Error::Plan(error))) if context_fields == 0 => {
+                        assert_eq!(&error, expected)
+                    }
+                    (expected, opened) => {
+                        panic!("an empty opening checked {opened:?}, not {expected:?}")
+                    }
+                }
+            }
         }
         Err(Error::Plan(error)) => {
             assert!(

packages/stack-encrypt/src/plan/build.rs

@@ -1324,6 +1324,13 @@ impl<S: 'static, K: 'static> Plan<S, K> {
         }
     }
 
+    /// The context the call names against the plan's own source, apart
+    /// from any value: exactly one of them. A batch asks this once, so an
+    /// empty one is refused as one value would be.
+    fn check_call(&self, call: Option<&Label>) -> Result<(), Error> {
+        Ok(self.base(call.cloned()).map(drop)?)
+    }
+
     /// A value against the plan, as running it checks it under the context
     /// the call names: one context source, the value's fields are the
     /// plan's (when it reads any by name), each is of the type the plan
@@ -1352,6 +1359,20 @@ impl<S: 'static, K: 'static> Plan<S, K> {
         self.opening_base(record, call.cloned()).map(drop)
     }
 
+    /// The context the call names against the plan's own source, as
+    /// opening checks it apart from any record: exactly one, for a plan
+    /// that does not read its context from a field. For one that does, the
+    /// call names what the record's field should hold, which only a record
+    /// can answer. A batch asks this once, so an empty one is refused as
+    /// one record would be.
+    fn check_opening_call(&self, call: Option<&Label>) -> Result<(), Error> {
+        match &self.inner.source {
+            Source::Field(_) => Ok(()),
+            Source::Plan(label) => Ok(resolve_context(Some(label), call.cloned()).map(drop)?),
+            Source::Call => Ok(resolve_context(None, call.cloned()).map(drop)?),
+        }
+    }
+
     /// The context a stored record opens under.
     fn opening_base(&self, record: &FieldValues, call: Option<Label>) -> Result<Label, Error> {
         match (&self.inner.source, call) {
@@ -1545,9 +1566,11 @@ impl<S: 'static, K: 'static> Runs<S, K> for Plan<S, K> {
 }
 
 /// A collection of sources runs one description per item, merged into one
-/// batch.
+/// batch. The context the call names is checked once, by the plan's
+/// `$check_call`, before any item: an empty collection runs no item, and
+/// would otherwise accept a context missing or given twice.
 macro_rules! runs_over_collections {
-    ($([$($generics:tt)*] $plan:ty => $item:ty where [$($bounds:tt)*];)+) => {$(
+    ($([$($generics:tt)*] $plan:ty => $item:ty where [$($bounds:tt)*] $check_call:ident;)+) => {$(
         impl<$($generics)*> Runs<[$item], K> for $plan where $($bounds)* {
             type Output = Vec<<Self as Runs<$item, K>>::Output>;
             fn pending<'p>(
@@ -1557,6 +1580,9 @@ macro_rules! runs_over_collections {
                 context: Option<Label>,
                 extend: DeclaredContext,
             ) -> Pending<'p, Self::Output, K> {
+                if let Err(error) = self.$check_call(context.as_ref()) {
+                    return Pending::failed(keyset, error);
+                }
                 Pending::all(
                     keyset,
                     source
@@ -1574,6 +1600,7 @@ macro_rules! runs_over_collections {
                 )
             }
             fn check(&self, source: &[$item], context: Option<&Label>) -> Result<(), Error> {
+                self.$check_call(context)?;
                 source
                     .iter()
                     .try_for_each(|item| Runs::<$item, K>::check(self, item, context))
@@ -1598,7 +1625,7 @@ macro_rules! runs_over_collections {
 }
 pub(crate) use runs_over_collections;
 runs_over_collections! {
-    [S, K] Plan<S, K> => S where [S: 'static, K: 'static];
+    [S, K] Plan<S, K> => S where [S: 'static, K: 'static] check_call;
 }
 
 impl<S: 'static, K: 'static> Opens<FieldValues, K> for Plan<S, K> {
@@ -1624,13 +1651,17 @@ impl<S: 'static, K: 'static> Opens<Vec<FieldValues>, K> for Plan<S, K> {
         context: Option<Label>,
         extend: DeclaredContext,
     ) -> Decryption<Vec<FieldValues>, K> {
+        if let Err(error) = self.check_opening_call(context.as_ref()) {
+            return Decryption::failed(error);
+        }
         Decryption::all(
             records
                 .into_iter()
                 .map(|record| Plan::decryption(self, record, context.clone(), extend.clone())),
         )
     }
     fn check(&self, records: &Vec<FieldValues>, context: Option<&Label>) -> Result<(), Error> {
+        self.check_opening_call(context)?;
         records
             .iter()
             .try_for_each(|record| self.check_opening(record, context))

packages/stack-encrypt/src/plan/value.rs

@@ -315,6 +315,13 @@ impl<S, X> ValuePlan<S, X> {
         resolve_context(self.context.as_ref(), call)
     }
 
+    /// The context the call names against the plan's own: exactly one of
+    /// them, which is all a one-value plan checks before a run or an
+    /// opening.
+    fn check_call(&self, call: Option<&Label>) -> Result<(), Error> {
+        Ok(self.resolve(call.cloned()).map(drop)?)
+    }
+
     /// The description one run of this plan executes, given the context the
     /// call names (for a plan built without one; `None` otherwise):
     /// the layout `.under(context)`. A context missing or given twice is a
@@ -375,12 +382,12 @@ where
     }
 
     fn check(&self, _source: &S, context: Option<&Label>) -> Result<(), Error> {
-        Ok(self.resolve(context.cloned()).map(drop)?)
+        self.check_call(context)
     }
 }
 
 runs_over_collections! {
-    [S, X, K] ValuePlan<S, X> => S where [X: ValueShape<S>, K: 'static];
+    [S, X, K] ValuePlan<S, X> => S where [X: ValueShape<S>, K: 'static] check_call;
 }
 
 impl<S, X> ValuePlan<S, X> {
@@ -444,7 +451,7 @@ macro_rules! indexed_opens {
                 self.open_one(record, context, extend)
             }
             fn check(&self, _: &$record, context: Option<&Label>) -> Result<(), Error> {
-                Ok(self.resolve(context.cloned()).map(drop)?)
+                self.check_call(context)
             }
         }
         impl<S, X, K, $($generics)*> Opens<Vec<$record>, K> for ValuePlan<S, Indexed<X>>
@@ -462,7 +469,7 @@ macro_rules! indexed_opens {
                 self.open_all(records, context, extend)
             }
             fn check(&self, _: &Vec<$record>, context: Option<&Label>) -> Result<(), Error> {
-                Ok(self.resolve(context.cloned()).map(drop)?)
+                self.check_call(context)
             }
         }
     )+};
@@ -491,7 +498,7 @@ where
         self.open_one(record, context, extend)
     }
     fn check(&self, _: &T, context: Option<&Label>) -> Result<(), Error> {
-        Ok(self.resolve(context.cloned()).map(drop)?)
+        self.check_call(context)
     }
 }
 
@@ -512,6 +519,6 @@ where
         self.open_all(records, context, extend)
     }
     fn check(&self, _: &Vec<T>, context: Option<&Label>) -> Result<(), Error> {
-        Ok(self.resolve(context.cloned()).map(drop)?)
+        self.check_call(context)
     }
 }