fix: secret_providers config must be addictive

Author: Marco Napetti

crates/firma-run/src/config.rs

@@ -512,7 +512,16 @@ impl Merge for ProfilePatch {
             allow_non_structural: higher.allow_non_structural.or(self.allow_non_structural),
             mask_home_paths: higher.mask_home_paths.or(self.mask_home_paths),
             ca_trust_mode: higher.ca_trust_mode.or(self.ca_trust_mode),
-            secret_providers: higher.secret_providers.or(self.secret_providers),
+            secret_providers: match (self.secret_providers, higher.secret_providers) {
+                // Additive across layers (like `env_passthrough`); the later
+                // (higher) entries come last so they win on name collision in
+                // `resolve_secret_providers`.
+                (Some(mut lower), Some(higher)) => {
+                    lower.extend(higher);
+                    Some(lower)
+                }
+                (lower, higher) => higher.or(lower),
+            },
         }
     }
 }

crates/firma-run/src/secret/accept.rs

@@ -96,15 +96,43 @@ mod tests {
         (listener, addr)
     }
 
+    fn echo_spec() -> firma_secret_provider::spec::cli::CliIntegrationSpec<firma_core::SecretMatcher>
+    {
+        use firma_secret_provider::{
+            non_empty::NonEmptyVec,
+            spec::{MatcherRule, cli::CommandPattern},
+        };
+
+        // Exact `SafeCommand` on the whole argv so `echo` is classified as a
+        // known-safe passthrough (unconfigured binaries are rejected now).
+        firma_secret_provider::spec::cli::CliIntegrationSpec::new(
+            "echo".to_string(),
+            "echo".to_string(),
+            vec![],
+            vec![],
+            vec![],
+            vec![MatcherRule::SafeCommand(CommandPattern::exact(
+                NonEmptyVec::new(vec![
+                    String::from("hello"),
+                    String::from("from"),
+                    String::from("broker"),
+                ])
+                .expect("non-empty argv"),
+            ))],
+        )
+        .unwrap_or_else(|error| panic!("valid spec: {error}"))
+    }
+
     #[tokio::test]
     async fn echo_end_to_end() {
         let (listener, addr) = bind_tcp().await;
         let store = Arc::new(RwLock::new(SecretStore::new()));
 
+        let spec = echo_spec();
         let server = tokio::spawn(serve_forever(
             listener,
             Arc::clone(&store),
-            Arc::new(|_bin: &str| None),
+            Arc::new(move |_bin: &str| Some(spec.clone())),
             None,
         ));
 

crates/firma-run/src/secret/serve.rs

@@ -287,14 +287,42 @@ mod tests {
         RwLock::new(SecretStore::new())
     }
 
+    fn echo_spec() -> firma_secret_provider::spec::cli::CliIntegrationSpec<SecretMatcher> {
+        use firma_secret_provider::{
+            non_empty::NonEmptyVec,
+            spec::{MatcherRule, cli::CommandPattern},
+        };
+
+        // An exact `SafeCommand` match on the whole argv: passthrough with no
+        // extraction, the only execution path still reachable for `echo` now
+        // that unconfigured binaries are rejected.
+        firma_secret_provider::spec::cli::CliIntegrationSpec::new(
+            "echo".to_string(),
+            "echo".to_string(),
+            vec![],
+            vec![],
+            vec![],
+            vec![MatcherRule::SafeCommand(CommandPattern::exact(
+                NonEmptyVec::new(vec![
+                    String::from("hello"),
+                    String::from("from"),
+                    String::from("broker"),
+                ])
+                .expect("non-empty argv"),
+            ))],
+        )
+        .unwrap_or_else(|error| panic!("valid spec: {error}"))
+    }
+
     #[tokio::test]
-    async fn passthrough_without_spec_returns_raw_stdout() {
+    async fn passthrough_safe_command_returns_raw_stdout() {
         let store = store();
         let request = BrokerRequest {
             bin: BinaryName::new("echo").expect("valid bin"),
             args: vec!["hello".into(), "from".into(), "broker".into()],
         };
-        let response = serve_request(&request, None, &store, None, CAPTURE_LIMIT).await;
+        let spec = echo_spec();
+        let response = serve_request(&request, Some(&spec), &store, None, CAPTURE_LIMIT).await;
         let decoded = response.decode().expect("decode");
         match decoded {
             firma_secret_provider::broker::DecodedBrokerResponse::Executed(output) => {
@@ -315,6 +343,24 @@ mod tests {
         }
     }
 
+    #[tokio::test]
+    async fn unconfigured_binary_is_rejected() {
+        let store = store();
+        let request = BrokerRequest {
+            bin: BinaryName::new("echo").expect("valid bin"),
+            args: vec!["hello".into()],
+        };
+        let response = serve_request(&request, None, &store, None, CAPTURE_LIMIT).await;
+        assert!(
+            matches!(
+                response.decode().expect("decode"),
+                firma_secret_provider::broker::DecodedBrokerResponse::Rejected(reason)
+                    if reason.contains("unconfigured binary echo")
+            ),
+            "a binary with no resolved spec must be rejected, not executed"
+        );
+    }
+
     #[tokio::test]
     async fn blocked_forbidden_option_is_rejected() {
         use firma_config_schema::secret_provider::cli::FlagSpec;
@@ -423,8 +469,9 @@ mod tests {
             bin: BinaryName::new("echo").expect("valid bin"),
             args: vec!["hello".into(), "from".into(), "broker".into()],
         };
+        let spec = echo_spec();
         let tiny_limit = 4usize;
-        let response = serve_request(&request, None, &store, None, tiny_limit).await;
+        let response = serve_request(&request, Some(&spec), &store, None, tiny_limit).await;
         assert!(
             matches!(
                 response.decode().expect("decode"),