fix: exempt loopback from TierAuth so local vault run doesn't get 429'd#97
Merged
dangtony98 merged 2 commits intomainfrom Apr 21, 2026
Merged
fix: exempt loopback from TierAuth so local vault run doesn't get 429'd#97dangtony98 merged 2 commits intomainfrom
dangtony98 merged 2 commits intomainfrom
Conversation
The CA-fetch path (`/v1/mitm/ca.pem`) and other public static reads were gated by per-IP TierAuth (10 req / 5 min). `vault run` calls the CA endpoint on every invocation, and the dashboard re-fetches it on component re-mount — both from 127.0.0.1, so a developer iterating on `vault run` could easily burn the budget and fall back to the explicit-proxy-only path with a confusing 429 log line. - Add `ratelimit.IPKeySkipLoopback` so loopback peers return an empty key and the middleware fails open (mirrors the existing MITM CONNECT policy in internal/mitm/connect.go). - Drop the `ipAuth` wrapper from `/v1/service-catalog`, `/v1/skills/*`, and `/v1/mitm/ca.pem`. These are immutable static reads with no credentials on the wire — TierGlobal is the only useful backstop. Login/register keep their own in-handler IP+email buckets, so credential-bearing endpoints retain defense-in-depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
4 tasks
Review on #97 surfaced two real issues with the IPKeySkipLoopback approach: 1. Exempting loopback at the keyer level also silently exempts /v1/auth/register, /v1/auth/verify, /v1/auth/resend-verification, /v1/auth/forgot-password, and /v1/auth/reset-password. Only handleLogin enforces an in-handler IP bucket, so a local caller could spam registrations or reset emails unbounded. 2. IPKeySkipLoopback checks the XFF-derived clientIP rather than the direct TCP peer. A misconfigured trusted proxy (common nginx footgun) lets a remote attacker forge X-Forwarded-For: 127.0.0.1 and bypass rate limiting entirely. The route unwrap on the four public static reads (/v1/service-catalog, /v1/skills/cli, /v1/skills/http, /v1/mitm/ca.pem) already solves the reported vault run regression — the CA fetch is the only ipAuth route the CLI hits. Revert the keyer and the ratelimit helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Both review findings addressed in 774b734 — dropping the keyer change entirely.
The route unwrap on the four public static reads ( Final diff vs main is 10 insertions / 8 deletions in |
dangtony98
added a commit
that referenced
this pull request
Apr 21, 2026
Resolved conflict in internal/server/server.go: #97 (merged) dropped the ipAuth wrapper from /v1/service-catalog, /v1/skills/{cli,http}, and /v1/mitm/ca.pem. Kept that change and preserved this branch's new /v1/vaults/{name}/logs route alongside it. Also applied De Morgan's to two QF1001 staticcheck findings in the new TestListRequestLogsTailOrdering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dangtony98
added a commit
that referenced
this pull request
Apr 21, 2026
## Summary
- New per-vault request log captures every proxy and transparent-MITM
request (actor, matched service, credential keys injected, status,
latency), surfaced via a Logs tab in the dashboard and `GET
/v1/vaults/{name}/logs`.
- MITM and explicit-proxy paths share a single `requestlog.Sink`, so
both ingress paths feed the same pipeline with consistent fields.
- Background retention worker trims by age and per-vault row cap; knobs
exposed as env vars and CLI flags.
## Changes
**Store**
- Migration
[039_request_logs.sql](internal/store/migrations/039_request_logs.sql)
adds the `request_logs` table + two indexes.
- CRUD in [internal/store/sqlite.go](internal/store/sqlite.go); methods
added to the `Store` interface.
**Pipeline**
- New [internal/requestlog/](internal/requestlog/) package: `Sink`
interface, SQLite-backed implementation, retention worker.
- `AttachLogSink` on the Server wires the sink at startup; the
explicit-proxy and MITM handlers fan events into it.
**API + docs**
- `GET /v1/vaults/{name}/logs` (vault-member auth) returns paginated
logs.
- Skill files ([cmd/skill_cli.md](cmd/skill_cli.md),
[cmd/skill_http.md](cmd/skill_http.md)) updated.
- Env-var / CLI-flag reference updated ([.env.example](.env.example),
[docs/reference/cli.mdx](docs/reference/cli.mdx),
[docs/self-hosting/environment-variables.mdx](docs/self-hosting/environment-variables.mdx)).
**UI**
- [web/src/pages/vault/LogsTab.tsx](web/src/pages/vault/LogsTab.tsx) +
[web/src/components/LogsView.tsx](web/src/components/LogsView.tsx)
render the table with filtering and status/latency cells.
- `AllAgentsTab` and `router.tsx` updated to integrate the new layout.
## Test plan
- [x] `go test ./...` green
- [ ] Manual: start the broker, exercise both explicit `/proxy/*` and
MITM HTTPS_PROXY flows, confirm rows land in the Logs tab with correct
actor / status / latency / credential-keys fields
- [ ] Manual: verify retention — set a low per-vault cap via env,
generate > cap rows, confirm trim on next worker tick
- [ ] Manual: verify auth — non-vault-member gets 403 on `GET
/v1/vaults/{name}/logs`
## Notes
- Branched off `main`. Does not depend on #97 (rate-limit loopback fix)
— they touch disjoint hunks.
- Frontend changes ship as source; `make build` will re-embed `web/dist`
into the binary.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
vault runfetches/v1/mitm/ca.pemon every invocation and the dashboard re-fetches it on component re-mount — both from127.0.0.1. Per-IPTierAuth(10 req / 5 min) would trip after a handful of local iterations, and the CLI would fall back to explicit-proxy-only with a confusingserver returned 429log line.TierAuthat the keyer level (mirrors the existing MITM CONNECT policy in internal/mitm/connect.go). Addedratelimit.IPKeySkipLoopbackso the"ip:"prefix + loopback logic lives in one place.ipAuthwrapper from/v1/service-catalog,/v1/skills/cli,/v1/skills/http,/v1/mitm/ca.pem. These are immutable static reads with no credentials on the wire — same category as/healthand/v1/status, which already rely on the TierGlobal backstop.Login/register keep their own in-handler IP+email buckets (handle_auth.go:595), so credential-bearing endpoints retain defense-in-depth even from loopback.
Test plan
go test ./...green (existing MITM/service-catalog/skill handler tests unchanged)127.0.0.0/8, IPv6::1, and non-loopback keys./agent-vault vault run -- claude20+ times in a row with the dashboard open; confirm therouting HTTPS through MITM proxyline appears every time🤖 Generated with Claude Code