Review feedback: gateway forwarding/protocol robustness issues

[gudnuf]

Summary

I ran a full repo review and test pass (cargo test --workspace). Tests pass, but I found several high-confidence robustness issues worth addressing.

Findings

1. warning — crates/pops-gateway/src/gateway.rs:80-81,343

   • Upstream forwarding client follows redirects by default.
   • Proxy behavior is safer/more predictable if redirects are not auto-followed.
   • Suggested fix: set redirect(reqwest::redirect::Policy::none()) in build_upstream_client.

2. warning — crates/pops-gateway/src/gateway.rs:132,395-399 (doc mismatch at crates/pops-gateway/README.md:146)

   • Authorization is stripped for all paths, including Gate::Public.
   • This can break upstream auth passthrough and contradicts “original request … headers … forwarded”.
   • Suggested fix: preserve Authorization on public routes; on charged routes strip only Authorization: Payment ... (or explicitly document current behavior).

3. warning — crates/pops-gateway/src/gateway.rs:408-429

   • Hop-by-hop filtering is static-only; headers nominated by Connection are not dynamically removed.
   • Suggested fix: parse Connection tokens and remove named headers for both request and response forwarding.

4. warning — crates/pops-core-verify/src/envelope.rs:264

   • WWW-Authenticate param parser uses split(','), which fails on valid quoted values containing commas.
   • Suggested fix: implement quote-aware auth-param tokenization.

5. nit — crates/pops-gateway/src/health.rs:24 vs :42-49

   • Comment says readyz returns 200 on “any HTTP response”, implementation returns 503 on non-2xx.
   • Suggested fix: align comment and behavior.

Validation

• cargo test --workspace --manifest-path /Users/claude/pops/Cargo.toml ✅