icaptcha-client: no test binds to IcaptchaCfg::new's API-key gate, so a key-attach regression would ship green

[beardthelion]

IcaptchaCfg::new (crates/icaptcha-client/src/lib.rs) decides whether the operator's
GITLAWB_ICAPTCHA_API_KEY bearer token is attached to the solve, and attaches it only
when resolve_solver_url returns key_trusted (the resolved origin is the operator's own
GITLAWB_ICAPTCHA_URL, never a node-advertised one). That gate is what stops a hostile node
from redirecting the solve and capturing the bearer token. The pure key_trusted decision is
well-tested and correct; what has no test is new()'s wiring that consumes it.

The seven key-trust tests in the crate all call resolve_solver_url directly and assert on its
returned key_trusted bool; none construct through IcaptchaCfg::new, so nothing asserts that
new() actually honors key_trusted when it populates api_key. The other construction paths
do not cover it either: the one non-test caller, crates/gl/src/http.rs, reaches new() but
never sets GITLAWB_ICAPTCHA_API_KEY and never asserts on api_key; and PR #184 adds
crates/icaptcha-client/tests/icaptcha_mock_consumed.rs, which builds IcaptchaCfg by struct
literal with api_key: None, bypassing new() entirely. No test in the workspace sets
GITLAWB_ICAPTCHA_API_KEY.

Verified by mutation. The crate's source (lib.rs / pow.rs / solvers.rs) is byte-identical
across origin/main, the fix/issue-135-ipfs-cid-tree-gate branch, and PR #184's head; PR #184
additionally adds the tests/ file mentioned above. Running at origin/main:

• Baseline: the 18 icaptcha-client lib tests are green.
• Defect injected into new() (attach the key unconditionally, ignoring key_trusted): all 18
  still green. That is the gap. The mutation is exactly the regression that drops the gate, and
  the suite does not notice.

The exposure if such a regression landed: a wrongly-attached key is not inert. obtain_proof
sends it via req.bearer_auth(key) on both the challenge and answer requests, so a new() that
attached the key for a node-advertised origin would hand the operator's bearer token to that
origin. This is the credential-exfil half of the same SSRF boundary the resolve_solver_url
tests already guard on the pure-function side.

Suggested fix: add one guard test in lib.rs's #[cfg(test)] mod tests (so it shares the
existing ICAPTCHA_ENV_LOCK; a tests/ integration file would not see that lock and its env
mutations would race the sibling env-touching tests). It constructs through new() and asserts
both arms:

#[test]
fn new_withholds_key_from_untrusted_origin_but_attaches_for_operator() {
    let guard = ICAPTCHA_ENV_LOCK.lock().unwrap();
    let prev_key = std::env::var_os("GITLAWB_ICAPTCHA_API_KEY");
    let prev_op = std::env::var_os("GITLAWB_ICAPTCHA_URL");
    std::env::set_var("GITLAWB_ICAPTCHA_API_KEY", "secret-bearer");

    // No operator configured, node advertises an attacker origin: the key has
    // no trusted destination, so new() must NOT attach it.
    std::env::remove_var("GITLAWB_ICAPTCHA_URL");
    let from_untrusted =
        IcaptchaCfg::new("did:key:zTEST", Some("https://evil.example".into()), None).api_key;

    // Operator origin configured and advertised: the key is attached.
    std::env::set_var("GITLAWB_ICAPTCHA_URL", "https://icap.mynode.example");
    let from_operator =
        IcaptchaCfg::new("did:key:zTEST", Some("https://icap.mynode.example/v1".into()), None)
            .api_key;

    // Restore env and release the lock BEFORE asserting, so a failing assertion
    // can't poison the shared lock or cascade into a sibling test.
    match prev_key {
        Some(v) => std::env::set_var("GITLAWB_ICAPTCHA_API_KEY", v),
        None => std::env::remove_var("GITLAWB_ICAPTCHA_API_KEY"),
    }
    match prev_op {
        Some(v) => std::env::set_var("GITLAWB_ICAPTCHA_URL", v),
        None => std::env::remove_var("GITLAWB_ICAPTCHA_URL"),
    }
    drop(guard);

    assert_eq!(from_untrusted, None, "key must not ride to an untrusted origin");
    assert_eq!(from_operator.as_deref(), Some("secret-bearer"));
}

This test is load-bearing, verified both ways at origin/main: RED against the unconditional-key
mutation (the whole suite shows exactly one failure, on the from_untrusted assertion where
api_key is Some but must be None), green against the current code (19 lib tests pass). It
captures both results and restores env before asserting so a RED never poisons the shared lock.
The exact env-restore shape is a starting point, not a mandate; a temp-env-style helper would
read cleaner if the crate takes that on.

This is a coverage gap on a gate whose logic is correct today, not a live vulnerability, so it
does not block #184. Filing separately for the same reason #211 was.