test(icaptcha-client): bind new()'s empty-API-key filter (#216)

[beardthelion]

Summary

Adds a guard test that drives an empty GITLAWB_ICAPTCHA_API_KEY through IcaptchaCfg::new on a trusted operator origin and asserts it normalizes to None, with a contrast arm showing a real key still attaches.

Motivation & context

Closes #216

new() normalizes an empty key to None via .filter(|s| !s.is_empty()) (crates/icaptcha-client/src/lib.rs:213). No test exercised the empty-string value, so dropping the filter shipped green and a trusted-origin solve would send an empty bearer token on the challenge and answer requests (lib.rs:317, lib.rs:342).

Kind of change

• Bug fix
• Feature
• Security fix
• Docs
• Tests / CI
• Refactor (no behavior change)
• Breaking or protocol change (issue required first)

What changed

• crates/icaptcha-client/src/lib.rs: added new_normalizes_empty_api_key_to_none_on_trusted_origin

How a reviewer can verify

cargo test -p icaptcha-client

Mutation check: remove .filter(|s| !s.is_empty()) from the api_key read in new(). The new test fails; the other 21 lib tests stay green.

Before you request review

• Scope is one logical change; no unrelated churn
• cargo test --workspace passes locally
• New behavior is covered by tests (required for fixes)
• cargo clippy --workspace --all-targets -- -D warnings is clean
• Commit titles use Conventional Commits (test(...))
• Docs / .env.example updated if behavior or config changed (N/A: test only)
• Checked existing PRs so this isn't a duplicate (nearest is test(icaptcha-client): add test that new() honors key_trusted gate (#212) #213 for icaptcha-client: no test binds to IcaptchaCfg::new's API-key gate, so a key-attach regression would ship green #212, which covers the key_trusted gate, not the emptiness filter)

Protocol & signing impact

No protocol impact.

Notes for reviewers

The test follows the same env-handling pattern as the sibling tests in the file: it saves and restores both env vars and releases ICAPTCHA_ENV_LOCK before asserting, so a failure cannot poison the shared lock or cascade into a sibling.

Summary by CodeRabbit

• Tests
  • Added coverage confirming that an empty CAPTCHA API key is treated as unset for trusted origins.
  • Verified that non-empty API keys remain preserved.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91782455-fe51-40bf-8b51-235c39878a9f

📥 Commits

Reviewing files that changed from the base of the PR and between 111cff7 and f35d9a7.

📒 Files selected for processing (1)

• crates/icaptcha-client/src/lib.rs

---

📝 Walkthrough

Walkthrough

Adds a unit test covering empty API key normalization and preservation of non-empty keys for trusted iCaptcha origins, with environment isolation and restoration.

Changes

iCaptcha API key normalization coverage

Layer / File(s)	Summary
Trusted-origin API key normalization test crates/icaptcha-client/src/lib.rs	Adds coverage verifying an empty GITLAWB_ICAPTCHA_API_KEY becomes None, while a non-empty value remains present, with environment state restored after the test.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

• icaptcha-client: no test binds to IcaptchaCfg::new's API-key gate, so a key-attach regression would ship green #212 — Covers related IcaptchaCfg::new API key handling behavior.

Possibly related PRs

• Gitlawb/node#213 — Adds related IcaptchaCfg::new tests for origin-scoped API key behavior.
• Gitlawb/node#168 — Adds iCaptcha client coverage around environment-derived trust and API key handling.

Suggested labels: kind:test, kind:bug

Suggested reviewers: kevincodex1

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the PR's main change: a regression test for empty API-key normalization in icaptcha-client.
Description check	✅ Passed	The description follows the template and fills the required sections with a clear summary, motivation, change list, and verification steps.
Linked Issues check	✅ Passed	The added test matches #216 by covering empty API-key normalization, preserving a real key, and using the shared env lock.
Out of Scope Changes check	✅ Passed	The PR stays within scope by changing only the icaptcha-client test file and introducing no unrelated behavior changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/icaptcha-empty-key-filter

---

_{Comment @coderabbitai help to get the list of available commands.}

[jatmn]

LGTM

[beardthelion]

@kevincodex1 ready for merge. jatmn approved the current head (f35d9a7), CI is green. Test-only change: binds the empty-API-key filter in new() per #216.