cellos-broker-file 0.5.1

//! BFILE-VAL: pin path-traversal and symlink-resistance for `FileSecretBroker`.
//!
//! Threat model — `FileSecretBroker` reads a path from
//! `CELLOS_SECRET_FILE_<UPPER_KEY>` and returns the file contents. The two
//! attacker-reachable surfaces are:
//!
//! 1. The **secret key** itself (passed by the supervisor via the cell spec's
//!    `secretRefs`). It is upper-cased and concatenated into an env-var name,
//!    so it should never form a path. We still pin the empty / NUL / `..` /
//!    leading-`/` cases so a future refactor that started using the key as a
//!    path component (or a more permissive env-var lookup) cannot silently
//!    introduce traversal.
//! 2. The **configured file path** (operator-controlled, read from the env
//!    var). Even though only the operator writes the env var, a process with
//!    write access to the *parent directory* of that path can swap the file
//!    for a symlink to e.g. `/etc/shadow`. Without `O_NOFOLLOW` on the open,
//!    the broker would happily exfiltrate the symlink target and emit it in
//!    a `SecretView`. The Unix path now opens with `O_NOFOLLOW`; this test
//!    pins the property by constructing such a symlink and asserting that
//!    `resolve` errors instead of returning the target's bytes.
//!
//! Each test uses a unique env-var suffix (per-test fresh string) so the
//! cargo test harness's shared process env does not race.

use std::io::Write;

use cellos_broker_file::FileSecretBroker;
use cellos_core::ports::SecretBroker;
use tempfile::NamedTempFile;

/// Helper: build a fresh secret-key string per test so concurrent tests do
/// not collide on the shared process env var table.
fn unique_key(suffix: &str) -> String {
    use std::sync::atomic::{AtomicU64, Ordering};
    static N: AtomicU64 = AtomicU64::new(0);
    let n = N.fetch_add(1, Ordering::Relaxed);
    format!("BFILE_VAL_{suffix}_{n}")
}

#[tokio::test]
async fn rejects_empty_key() {
    let broker = FileSecretBroker::new();
    let err = broker
        .resolve("", "cell-empty", 60)
        .await
        .expect_err("empty key must be rejected");
    let msg = err.to_string();
    assert!(
        msg.contains("empty"),
        "empty-key error should mention emptiness: {msg}"
    );
}

#[tokio::test]
async fn rejects_key_with_nul_byte() {
    let broker = FileSecretBroker::new();
    let err = broker
        .resolve("FOO\0BAR", "cell-nul", 60)
        .await
        .expect_err("NUL-byte key must be rejected");
    let msg = err.to_string();
    assert!(
        msg.contains("NUL") || msg.contains("nul"),
        "NUL-key error should mention NUL: {msg}"
    );
}

#[tokio::test]
async fn rejects_key_with_dotdot_traversal() {
    // `..` in the key would only enter a path if the broker ever started using
    // the key as a path component. Today it forms an env-var name, which on
    // most platforms accepts dots, so the env var is simply unset and the
    // resolve fails with "env var not set". Pin that failure mode so it stays
    // attributable to a missing-config error rather than a successful read of
    // a fallback path.
    let broker = FileSecretBroker::new();
    let err = broker
        .resolve("../etc/passwd", "cell-traversal", 60)
        .await
        .expect_err("`../`-bearing key must not resolve");
    let msg = err.to_string();
    assert!(
        // Env-var lookup fails because the env var was never set.
        msg.contains("env var") || msg.contains("not set"),
        "dotdot key should fail at env-var lookup, got: {msg}"
    );
}

#[tokio::test]
async fn rejects_key_with_leading_slash_absolute_path() {
    // A leading `/` is meaningful only if the key is treated as a path. Today
    // it just produces an env-var name with a `/` in it, which `std::env::var`
    // refuses (or simply isn't set), so the resolve fails. Pin that nothing
    // ever turns it into a successful read of `/etc/passwd`.
    let broker = FileSecretBroker::new();
    let err = broker
        .resolve("/etc/passwd", "cell-abs", 60)
        .await
        .expect_err("absolute-path-shaped key must not resolve");
    // Whether the error is "env var name invalid" or "env var not set", the
    // outcome we care about is that no file was successfully read. The error
    // type is `CellosError::SecretBroker` either way.
    let msg = err.to_string();
    assert!(
        !msg.is_empty(),
        "absolute-path key must produce a non-empty error"
    );
}

#[tokio::test]
async fn rejects_key_with_forward_slash() {
    // Same surface as the leading-slash case — a `/` in a key would matter
    // only if it ever entered a `Path::join`. Pin the contract that it does
    // not.
    let broker = FileSecretBroker::new();
    let err = broker
        .resolve("foo/bar", "cell-slash", 60)
        .await
        .expect_err("slash-bearing key must not resolve");
    assert!(!err.to_string().is_empty());
}

#[cfg(unix)]
#[tokio::test]
async fn symlink_pointing_outside_base_is_not_followed() {
    // Construct: <tmpdir>/inside_secret  --symlink-->  <tmpdir>/outside/target
    //
    // Then point the broker's env var at `inside_secret`. With `O_NOFOLLOW`
    // on the open, the broker MUST refuse to follow the symlink and the
    // contents of `outside/target` (the would-be exfiltrated file) MUST NOT
    // appear in the returned secret value. Without the fix this test fails
    // because `tokio::fs::read_to_string` transparently follows the symlink
    // and exfiltrates the target.
    let tmp = tempfile::tempdir().expect("tempdir");
    let outside = tmp.path().join("outside");
    std::fs::create_dir(&outside).expect("mkdir outside");
    let target = outside.join("target");
    std::fs::write(&target, b"PWNED-SHOULD-NEVER-BE-RETURNED").expect("write target");

    let inside = tmp.path().join("inside_secret");
    std::os::unix::fs::symlink(&target, &inside).expect("symlink");

    let key = unique_key("SYMLINK_OUT");
    let env_var = FileSecretBroker::path_env_var(&key);
    std::env::set_var(&env_var, inside.to_str().unwrap());

    let broker = FileSecretBroker::new();
    let result = broker.resolve(&key, "cell-symlink", 60).await;
    std::env::remove_var(&env_var);

    let err = result.expect_err(
        "symlink at the configured path must NOT be followed — \
         O_NOFOLLOW should refuse the open and produce an error",
    );
    let msg = err.to_string();
    assert!(
        !msg.contains("PWNED-SHOULD-NEVER-BE-RETURNED"),
        "error must not embed the symlink target's bytes: {msg}"
    );
    // Sanity: the error should reference the broker layer.
    assert!(
        msg.contains("read secret file") || msg.contains("ELOOP") || msg.contains("symbolic"),
        "error should be a read failure attributable to the broker / O_NOFOLLOW: {msg}"
    );
}

#[cfg(unix)]
#[tokio::test]
async fn symlink_to_arbitrary_host_file_does_not_exfiltrate() {
    // Strongest property: even if the symlink points at a host file the
    // supervisor process can read (we use a tempfile with a known sentinel
    // so the test does not depend on /etc/passwd's contents or readability),
    // the broker must NOT return the target's bytes.
    let mut victim = NamedTempFile::new().expect("victim tempfile");
    write!(victim, "VICTIM-CONTENT-DO-NOT-LEAK").expect("write victim");

    let tmp = tempfile::tempdir().expect("tempdir");
    let inside = tmp.path().join("secret_via_symlink");
    std::os::unix::fs::symlink(victim.path(), &inside).expect("symlink");

    let key = unique_key("SYMLINK_HOST");
    let env_var = FileSecretBroker::path_env_var(&key);
    std::env::set_var(&env_var, inside.to_str().unwrap());

    let broker = FileSecretBroker::new();
    let result = broker.resolve(&key, "cell-host-symlink", 60).await;
    std::env::remove_var(&env_var);

    match result {
        Ok(view) => panic!(
            "symlink-follow must be refused, but resolve returned a value of len {}",
            view.value.as_str().len()
        ),
        Err(e) => {
            let msg = e.to_string();
            assert!(
                !msg.contains("VICTIM-CONTENT-DO-NOT-LEAK"),
                "error must not embed victim file's bytes: {msg}"
            );
        }
    }
}

#[cfg(unix)]
#[tokio::test]
async fn regular_file_outside_tempdir_still_works() {
    // Negative control: the O_NOFOLLOW fix must not regress the happy path.
    // A plain regular file (no symlink) at any absolute path the broker can
    // read must still resolve successfully.
    let mut f = NamedTempFile::new().expect("tempfile");
    write!(f, "happy-path-secret").expect("write");

    let key = unique_key("HAPPY");
    let env_var = FileSecretBroker::path_env_var(&key);
    std::env::set_var(&env_var, f.path().to_str().unwrap());

    let broker = FileSecretBroker::new();
    let view = broker.resolve(&key, "cell-happy", 60).await;
    std::env::remove_var(&env_var);

    let view = view.expect("regular file must still resolve after O_NOFOLLOW fix");
    assert_eq!(view.value.as_str(), "happy-path-secret");
}

#[cfg(unix)]
#[tokio::test]
async fn symlink_inside_tempdir_pointing_to_sibling_is_still_refused() {
    // Even when the symlink target is inside the same directory as the
    // configured path (i.e. nominally "in scope"), `O_NOFOLLOW` refuses the
    // final-component symlink. This documents that the broker enforces a
    // strict no-symlink-on-final-component policy, not a "stay inside dir"
    // policy — matching the SEC-15b convention used by the rest of the
    // workspace's trust loaders.
    let tmp = tempfile::tempdir().expect("tempdir");
    let real = tmp.path().join("real");
    std::fs::write(&real, b"sibling-content").expect("write");

    let link = tmp.path().join("link");
    std::os::unix::fs::symlink(&real, &link).expect("symlink");

    let key = unique_key("SIBLING_SYMLINK");
    let env_var = FileSecretBroker::path_env_var(&key);
    std::env::set_var(&env_var, link.to_str().unwrap());

    let broker = FileSecretBroker::new();
    let result = broker.resolve(&key, "cell-sibling", 60).await;
    std::env::remove_var(&env_var);

    let err = result.expect_err("final-component symlink must be refused even with in-dir target");
    assert!(
        !err.to_string().contains("sibling-content"),
        "error must not embed target bytes: {err}"
    );
}