Return 403 access denied for auth failures on public projects

[epompeii]

Summary

Previously, any project auth failure (anonymous, missing role, or a project
key scoped to a different project) responded with a generic 404 warning that
the project "may be private and require authentication or it may not exist."
That message exists so private projects do not leak their existence, but it
is actively misleading for public projects whose existence is already obvious
from the UI.

This PR makes the API return a clear 403 Forbidden with an "access denied"
explanation when the target project is provably public, while preserving the
info-hiding 404 for private/missing projects.

Changes

• lib/bencher_schema/src/error.rs — added project_auth_error(is_public, value, error) helper next to resource_not_found_error. Branches the response on visibility: 403 + "access denied" for public projects, existing 404 info-hiding wording otherwise.
• lib/bencher_schema/src/model/project/mod.rs — hoisted from_resource_id out of the three private auth helpers (is_allowed_inner, is_allowed_public, is_allowed_actor_inner) and into the public entry points (is_allowed, is_allowed_actor_pub, is_allowed_actor_auth) so query_project.is_public() is in scope when constructing the error. No extra DB query.
• Covers all three project auth paths: is_allowed (JWT + permission), is_allowed_actor_pub (read endpoints), and the project-key branch of is_allowed_actor_auth (write endpoints).
• The anonymous-on-private path inside is_allowed_public is unchanged in outward behavior — info hiding still applies.

Scenarios fixed

1. Authenticated user without Edit/Delete role tries to modify a public project → now 403 with "access denied" (was misleading 404 "may be private").
2. Request authenticates with a project API key scoped to project A but targets public project B → now 403 with "access denied" (was misleading 404 on reads, misleading 401 on writes).

Out of scope

Child-resource 404s (Benchmark, Branch, etc.) inherit the same misleading "may be private" wording. Fixing them would require plumbing &QueryProject into ~30 call sites or replacing each with a hand-written visibility branch. Left as a follow-up.

Test plan

• cargo test -p bencher_schema --features plus --lib error::tests — new unit tests for project_auth_error (forbidden for public, not-found with info-hiding for private)
• cargo test -p api_projects --test projects --features plus — 20/20, including new non_member_patch_public_project_returns_403 (JWT/role path) and non_member_patch_private_project_returns_404 (info hiding preserved)
• cargo test -p api_projects --test project_key_auth --features plus — 29/29, including new project_key_for_other_project_on_public_target_returns_403 (renamed) and project_key_for_other_project_on_private_target_returns_404
• cargo clippy --no-deps --all-targets --all-features -- -Dwarnings clean
• cargo check --no-default-features clean
• cargo fmt
• CI green

[github-actions]

🤖 Claude Code Review

PR: #868
Base: devel
Head: claude/public-project-auth-errors-ChpPo
Commit: bedccffab815e3d26777a91b1fc8dbd3555558a6

---

Now I have a thorough understanding of the changes. Here's my review:

---

PR Review: Project Auth Error Handling Refactor

Summary

This PR introduces a ProjectAuthState enum to systematically map (visibility x authentication) combinations to the correct HTTP status codes, replacing scattered ad-hoc error construction. It also adds thorough integration test coverage for all four auth/visibility combinations across multiple endpoints.

Findings

Positive

1. Strong security improvement. The ProjectAuthState enum makes the info-hiding invariant (private projects return 404, not 403/401) structurally enforced rather than convention-based. This is a significant defense against project-existence enumeration.

2. Good refactoring to &self methods. is_allowed_public, is_allowed_actor_inner no longer redundantly re-query the project from the DB — the from_resource_id call is done once in the public-facing method, then the inner method operates on &self. This is cleaner and avoids a potential TOCTOU-style double-fetch.

3. Excellent test coverage. The new tests cover all four quadrants (public/private x anon/authed) for reads (branches), writes (testbed create), and project patch, plus the cross-project-key scenarios. The #[cfg(feature = "plus")] gating on private-project tests is correct since visibility requires the plus feature.

4. is_allowed_actor_auth consolidation. The old code re-entered is_allowed() for the PublicUser::Auth arm, which did another from_resource_id query. The new code avoids this redundant DB call and centralizes rate limiting at the bottom.

Issues

1. Comment on is_allowed_public (minor, style)

lib/bencher_schema/src/model/project/mod.rs:443 — The comment // Private project + anonymous: outer caller wraps to info-hiding 404 is accurate but the CLAUDE.md says to default to no comments. This one is borderline; the comment documents a non-obvious security invariant (the unauthorized_error here does not surface as a 401 to the user). I'd keep it, but wanted to flag the tension.

2. ProjectAuthState doc comments vs. CLAUDE.md guidance (minor, style)

lib/bencher_schema/src/error.rs:216-261 — The doc comments on ProjectAuthState and its variants are well-written and justify themselves: they document a security invariant (the "why"). This is appropriate per CLAUDE.md guidelines.

3. Test comments in testbeds.rs (minor, style)

lib/api_projects/tests/testbeds.rs:572-574,595-597,630,647 — Several tests have multi-line comments explaining what code path they exercise (e.g., "Exercises is_allowed_actor_auth anonymous + public-project path"). These are useful for understanding coverage intent but are more descriptive than the typical project style. Consider whether the test names alone (which are already very descriptive, e.g. anonymous_create_testbed_on_public_project_returns_401) are sufficient.

4. Potential information leak in PublicAuthenticated error message (low risk)

lib/bencher_schema/src/error.rs:253-255 — The PublicAuthenticated arm includes "This {resource} is public but you do not have the required permission." This explicitly tells an attacker the project is public. This is fine for public projects (visibility is public information by definition), but worth noting that this message should never be reachable for private projects — and the enum structure guarantees it isn't.

5. resource_not_found_error is no longer used for the ProjectAuthState::auth_error import (nit)

lib/bencher_schema/src/model/project/mod.rs:19-22 — The old import included resource_not_found_error directly. It's now only used transitively through ProjectAuthState::auth_error. The import was correctly removed from the use statement.

Verified

• ProjectKey actors always return is_auth() == true, so a project key targeting a private project it doesn't belong to correctly gets the PrivateAuthenticated → 404 path.
• is_allowed_actor_auth for anonymous users early-returns with auth_error() before reaching try_allowed, preventing an anonymous user from falling through to an RBAC check that would panic or produce wrong errors.
• Rate limiting (project_request) is still applied in all write paths — the consolidation at the bottom of is_allowed_actor_auth covers both Auth and ProjectKey arms.

Verdict

Clean, well-tested security hardening. The ProjectAuthState enum is a good pattern that prevents an entire class of information-disclosure bugs. No blocking issues found — the style nits on test comments are optional.

---

_{Model: claude-opus-4-6}

[github-actions]

Bencher Report

Branch	claude/public-project-auth-errors-ChpPo
Testbed	intel-v1

Click to view all benchmark results

Benchmark	Latency	Benchmark Result microseconds (µs) (Result Δ%)	Upper Boundary microseconds (µs) (Limit %)
Adapter::Json	📈 view plot 🚷 view threshold	4.62 µs (+1.12%) Baseline: 4.57 µs	4.74 µs (97.49%)
Adapter::Magic (JSON)	📈 view plot 🚷 view threshold	4.48 µs (+0.77%) Baseline: 4.45 µs	4.60 µs (97.45%)
Adapter::Magic (Rust)	📈 view plot 🚷 view threshold	25.66 µs (+1.04%) Baseline: 25.40 µs	26.25 µs (97.77%)
Adapter::Rust	📈 view plot 🚷 view threshold	3.48 µs (+0.75%) Baseline: 3.46 µs	3.55 µs (98.05%)
Adapter::RustBench	📈 view plot 🚷 view threshold	3.49 µs (+1.15%) Baseline: 3.46 µs	3.55 µs (98.58%)

🐰 View full continuous benchmarking report in Bencher