feat: adversarial review

Author: Marco Napetti

.github/workflows/installer-smoke.yml

@@ -36,6 +36,7 @@ jobs:
           ./install.sh --install-dir "${INSTALL_DIR}"
           "${INSTALL_DIR}/firma" --version
           "${INSTALL_DIR}/firma" help config
+          test -x "${INSTALL_DIR}/firma-secret-shim"
       - name: Re-run (already installed exit path)
         env:
           FIRMA_NO_BREW: '1'
@@ -81,6 +82,7 @@ jobs:
           .\install.ps1 -InstallDir $dir
           & (Join-Path $dir 'firma.exe') --version
           & (Join-Path $dir 'firma.exe') help config
+          if (-not (Test-Path (Join-Path $dir 'firma-secret-shim.exe'))) { exit 1 }
       - name: Re-run (already installed exit path)
         shell: pwsh
         env:

crates/firma-run/src/config.rs

@@ -691,6 +691,22 @@ pub(crate) fn resolve_profile_with_layout(
         );
     }
 
+    // CLI secret providers are mediated by bind-mounting the
+    // `firma-secret-shim` binary over the real tool and routing it to a
+    // host-side broker socket. Only the Linux bwrap backend implements both
+    // the mount and the host-socket reachability; on any other backend the
+    // real tool would run unmediated, so fail closed instead.
+    let has_cli_providers = resolved
+        .secret_providers
+        .values()
+        .any(|spec| spec.as_cli().is_some());
+    if has_cli_providers && resolved.backend != BackendKind::Bwrap {
+        return Err(RunError::ConfigValidation(format!(
+            "secret_providers with a CLI entry is unsupported for backend '{}'; CLI secret mediation requires backend 'bwrap'. Remove the CLI entries or use an HTTP provider",
+            resolved.backend
+        )));
+    }
+
     resolved.validate()?;
     Ok(resolved)
 }
@@ -2862,6 +2878,59 @@ fail_closed = true
         );
     }
 
+    #[test]
+    fn cli_secret_providers_on_non_bwrap_backend_are_rejected() {
+        let tmpdir = tempfile::tempdir().unwrap();
+        let config_path = tmpdir.path().join(CONFIG_FILE_NAME);
+        let backend = non_bwrap_backend_for_current_host();
+        let toml = format!(
+            r#"
+[run.profiles.generic]
+backend = "{backend}"
+
+[run.defaults]
+secret_providers = ["bws"]
+"#
+        );
+        fs::write(&config_path, toml).unwrap();
+
+        let mut run_args = args("generic");
+        run_args.config = Some(config_path);
+
+        let error = resolve_profile(&run_args).expect_err("expected validation error");
+        assert!(
+            error.to_string().contains("CLI secret mediation requires backend 'bwrap'"),
+            "unexpected error: {error}"
+        );
+    }
+
+    #[test]
+    fn http_secret_providers_are_allowed_on_non_bwrap_backends() {
+        let tmpdir = tempfile::tempdir().unwrap();
+        let config_path = tmpdir.path().join(CONFIG_FILE_NAME);
+        let backend = non_bwrap_backend_for_current_host();
+        let toml = format!(
+            r#"
+[run.profiles.generic]
+backend = "{backend}"
+
+[run.defaults]
+secret_providers = [
+  {{ type = "http", provider_id = "aws-secrets-manager", host = "secretsmanager.*.amazonaws.com", matchers = [
+    {{ type = "sensitive_command", path = "/GetSecretValue", matcher = {{ type = "json", record_path = "$", value_path = "$.SecretString", name = {{ source = "path", path = "$.Name" }} }} }},
+  ] }},
+]
+"#
+        );
+        fs::write(&config_path, toml).unwrap();
+
+        let mut run_args = args("generic");
+        run_args.config = Some(config_path);
+
+        let resolved = resolve_profile(&run_args).expect("HTTP provider must not be backend-gated");
+        assert!(resolved.secret_providers.contains_key("aws-secrets-manager"));
+    }
+
     #[test]
     #[cfg_attr(not(target_os = "linux"), ignore = "bwrap-only")]
     fn seccomp_policy_resolves_when_configured_for_bwrap() {

crates/firma-run/src/runtime/mod.rs

@@ -30,7 +30,7 @@ pub mod vscode;
 
 /// Secret-mediation shim injection (Unix/bwrap). Wires the broker + shim mounts
 /// into a launch when the profile lists `secret_providers`.
-mod secret_shims;
+pub mod secret_shims;
 
 /// Lib-level input for [`execute_run`]. The CLI layer (in the `firma`
 /// host crate) builds this from its `clap`-derived args struct.
@@ -159,6 +159,11 @@ pub fn execute_run(args: &RunInput, hooks: &LaunchHooks<'_>) -> Result<i32, RunE
         profile: profile.clone(),
         working_dir: working_dir.clone(),
     })?);
+    // Secret services (gateway + broker) for this run. Assigned once the
+    // profile's providers are known inside the run closure below; dropped
+    // only after sandbox teardown so the agent can resolve placeholders for
+    // its whole lifetime while the plaintext store still cannot outlive the run.
+    let mut secret_services = None;
 
     let run_result = (|| {
         let handle_ref = handle
@@ -237,10 +242,21 @@ pub fn execute_run(args: &RunInput, hooks: &LaunchHooks<'_>) -> Result<i32, RunE
         let firma_exe = env::current_exe()
             .map_err(|e| RunError::Internal(format!("resolve current_exe: {e}")))?;
         // Start the secret services before the Sidecar so its gateway address
-        // is available while the Sidecar config is synthesized.
-        let gateway_binding = secret_shims::pre_bind_gateway(handle_ref, &profile)?;
-        if let Some(binding) = &gateway_binding {
-            flags.secret_gateway_addr = Some(binding.addr.clone());
+        // is available while the Sidecar config is synthesized. The guard is
+        // held for the whole run: dropping it stops the gateway and broker
+        // and removes their sockets.
+        secret_services = if profile.secret_providers.is_empty() {
+            None
+        } else {
+            Some(secret_shims::SecretServices::start(
+                &runtime_layout,
+                handle_ref,
+                &identity,
+                &profile,
+            )?)
+        };
+        if let Some(services) = &secret_services {
+            flags.secret_gateway_addr = Some(services.gateway_addr.clone());
         }
         let mut prompt = crate::authority::StdAuthorityPrompt;
         let authority = crate::routing::resolve_authority(
@@ -340,7 +356,7 @@ pub fn execute_run(args: &RunInput, hooks: &LaunchHooks<'_>) -> Result<i32, RunE
                 &mut env,
                 &firma_exe,
                 env::var_os("PATH").as_deref(),
-                gateway_binding.as_ref(),
+                secret_services.as_ref(),
             )?;
             let launch = LaunchSpec {
                 executable,
@@ -382,6 +398,11 @@ pub fn execute_run(args: &RunInput, hooks: &LaunchHooks<'_>) -> Result<i32, RunE
         .take()
         .map_or(Ok(()), |real_handle| backend.teardown(real_handle));
 
+    // Stop the secret services after sandbox teardown so the agent can still
+    // resolve placeholders while it exits, but before the run returns: the
+    // gateway socket is removed and the plaintext per-run store is dropped.
+    drop(secret_services);
+
     combine_run_and_teardown_results(run_result, teardown_result)
 }