p2.5: redact reason/err for permit_restricted logs

Guard messages embed the hostname (resolve <host>, no addresses for
<host>). Restricted callers now log a coarse reason class and err=-.
Test dumps slog records and forbids the hostname anywhere in them.

docs: P3 review — fail-closed search, cache class, public dialer
This commit is contained in:
2026-09-23 11:47:37 -07:00
parent 2d376d3ad1
commit 7be4ed1d43
4 changed files with 148 additions and 41 deletions
+3 -1
View File
@@ -150,7 +150,9 @@ context calls/hits/failures; recover attempts/successes by route; token ratio hi
Logs: one INFO line per request with `caller host mode q_len raw_tokens returned_tokens
digest_ms fallback`. Never log page content, questions longer than 80 chars, or keys.
Callers in `context.permit_restricted` log `host=-` on scrape, fetch, refused, fetch-failed,
and internal lines (P2.5). `final_url` stays in the response body.
and internal lines (P2.5). Their `reason` is a coarse class (`blocked`, `timeout`, …) and
`err` is `-`, so guard messages like `blocked: resolve <hostname>` do not reach the journal.
`final_url` stays in the response body.
## 7. Tests
+61 -30
View File
@@ -75,8 +75,11 @@ ignored (with a warning if `digest: true`). `digest` defaults **false** and stay
## 3. Sources
Config, not the request. Each source has `enabled` and a fixed endpoint. Missing key or
Config, not the request. Each source has `enabled` and a fixed endpoint.
`enabled = false` → source is skipped, no warning.
A source that is **enabled but missing its key** is not skipped silently: log once at
startup, and `/healthz` lists it as degraded (`search.<name>: missing key`). Requests
still run the other sources and add `warnings: <name>: missing key`.
```toml
[search]
@@ -112,8 +115,10 @@ the shared fan-in. This is the Hermes SearXNG provider's request (`plugins/web/s
PLAN §2, verified earlier: `GET /res/v1/llm/context?q=…` with `BRAVE_API_KEY` from
`/var/lib/sift/secret.env` (0640 root:sift). **Do not read SearXNG's secret.env.**
Header `X-Subscription-Token`. Map `sources[]` / `grounding` URLs + snippets; drop the
model-written `grounding` answer text if present — we want sources, not Brave's summary.
Header `X-Subscription-Token`. Map result URLs and snippets from the live JSON
(`sources` and any URL-bearing fields). Do **not** assume a model-written
`grounding` answer exists — confirm against a live response before dropping or
keeping any field that is not a URL/title/snippet.
This is **not** Brave Web Search (`/res/v1/web/search`). SearXNG already runs `braveapi`
as an engine; doubling that is wasted spend.
@@ -123,7 +128,8 @@ as an engine; doubling that is wasted spend.
- Key only in `/var/lib/sift/secret.env` as `TAVILY_API_KEY`. Never in chat, logs, metrics
labels, or the Nix store.
- `POST {url}` JSON: `query`, `max_results` (min(k, 20)), `include_answer: false`,
`include_raw_content: false`, `include_images: false`. `Authorization: Bearer`.
`include_raw_content: false`, `include_images: false`, `search_depth: "basic"`
(explicit, so credit cost is fixed). `Authorization: Bearer`.
- **Never request or return `answer`.** If Tavily still sends one, drop it before the
fan-in. Do not call `/extract`, `/crawl`, or `/map`.
- Map `results[].url|title|content` + `score`. Count `usage.credits` toward the daily cap.
@@ -145,7 +151,10 @@ placeholder line the same way (empty, on-box fill).
affect rerank input if they brought a longer snippet (keep the longer one).
5. Rerank (§5) unless `rerank: false` or Dixie cannot take the call.
6. Truncate to `k` (native) or `limit` (`/v2/search`, default 5, max 20).
7. Store in the search cache. Return.
7. Store in the search cache **only if** every attempted source succeeded and
rerank did not fall back. A timeout or fallback must not pin a partial result
for the rest of the hour (skip the write, or use a short TTL such as 2 m — skip
is the default). Return.
Search **never GETs a result URL**. Agents `sift fetch` those through the P1 guard.
@@ -170,11 +179,14 @@ only**:
1. Parse the config URL: `http`/`https` only, no userinfo. SearXNG may be loopback;
Brave and Tavily must be public hostnames (`api.search.brave.com`, `api.tavily.com`).
2. Resolve that hostname once (and refresh on a long interval, e.g. 10 m). The dialer
`Control` hook allows **only those resolved IPs, only that port**. Anything else,
including a rebinding answer, is refused.
3. No cookies, no proxy env, no redirects to a different host (max 1 hop, same host).
Redirect to a different host → source failure, not a follow.
2. Resolve that hostname once (and refresh on a long interval, e.g. 10 m). **A source
marked public must resolve only to public addresses** — run the guard's public-IP
check (`CheckAddr`) on every record. A poisoned `api.tavily.com → 127.0.0.1` is
refused. The dialer `Control` hook allows **only those resolved IPs, only that port**.
Anything else, including a rebinding answer, is refused.
3. TLS against **system roots**, no override, no `InsecureSkipVerify`. No cookies, no
proxy env, no redirects to a different host (max 1 hop, same host). Redirect to a
different host → source failure, not a follow.
4. Request path/query are code-built (`/search?q=&format=json`, `/res/v1/llm/context?q=`,
`POST /search`). Query text is query-escaped; it never becomes a URL.
@@ -190,11 +202,17 @@ different loopback port.
SQLite, same `/var/lib/sift/cache.db` as P2 digest cache, **new table** `search` (not
the digest/embed tables).
- Key: `sha256(norm(query) | sources | k | hour_bucket)`.
- Key: `sha256(norm(query) | sources | k | hour_bucket | caller_class)`.
`norm` = trim, collapse whitespace, lowercase (same as digest questions).
`sources` = sorted list of enabled source names that were actually attempted (not
the ones skipped for missing keys).
`hour_bucket` = UTC `YYYY-MM-DDTHH` of the request.
`caller_class` = `restricted` if the caller is in `permit_restricted`, else `fleet`.
This blocks a fleet caller from observing `cached: true` on a query only the
restricted caller issued this hour. Restricted callers currently fail closed
(§9) and never read or write this cache; the class is still in the key so a
later option (b)/(c) cannot reopen the side channel.
- Do not cache a result if any source failed or rerank fell back (§4 step 7).
- TTL: `search.cache_ttl` (default 1 h). Hour-bucket already bounds freshness; TTL is
the eviction/serve window if a process spans the hour mark.
- Value: URLs, titles, snippets, scores, source names. **No page bodies, no Tavily
@@ -204,6 +222,10 @@ the digest/embed tables).
The URL/fetch cache stays in P4.
**Follow-up (not P3):** the P2 digest cache is content-addressed by page hash +
question, so a weaker same-query side channel exists there too. Note it; do not
fix it in P3.
## 8. Cost, metrics, logs
`/metrics` (JSON, additive to P1/P2):
@@ -226,27 +248,31 @@ Logs: one INFO line `search caller=… q_len=… sources=… n=… cache=hit|mis
Never log the query text beyond `q_len`, never keys, never snippets. `permit_restricted`
callers: `host` is not a search field; do not log result hosts either.
## 9. Restricted callers — open question for Kyle
## 9. Restricted callers — fail closed until Kyle picks
Should `permit_restricted` callers' queries go to third-party engines (Brave, Tavily)
at all? SearXNG is on-box; Brave and Tavily see the query in vendor logs.
SearXNG is **not** private: on orion it forwards every query to bing, `braveapi`
(Kyle's Brave key), wikipedia, github and arxiv. Loopback only hides the hop, not
the vendor.
Do **not** decide this in implementation. Until Kyle answers, P3 code:
Kyle (2026-09-23) confirmed these searches are sensitive. Until he picks one of:
- still serves search for those callers
- still redacts hosts in fetch logs (P2.5)
- does **not** special-case their search queries
- ships a config knob `search.permit_restricted_paid = false` **commented, unused**,
so flipping it later is one line
- **(a)** restricted callers get no search from sift at all
- **(b)** a fixed, reduced SearXNG engine set for them
- **(c)** the same sources as the fleet
List this as unresolved in the review ping.
…the code **fails closed**: `403` `search not permitted` for `permit_restricted`
callers on `/v1/search` and `/v2/search`. No upstream call, no cache read or write,
no query text in logs. Skill: do not fall through to Hermes `web_search`.
Do not implement (b) or (c) on a guess.
## 10. Bake-off (acceptance)
A fixed set of **~30 real queries** harvested from recent fleet `web_search` use
(gateway/`agent.log` on straylight, talos, titan — the `web_search` / Tavily /
SearXNG lines). Deduped, no secrets, committed as `tests/testdata/search_queries.txt`.
Do not invent the set.
A fixed set of **~30 real queries** harvested from recent **fleet** `web_search` use
(gateway/`agent.log` on straylight, talos, titan). **Exclude** the guardian-star
profile and anything from `permit_restricted` callers. Deduped, no secrets. Kyle
sees the list before it is committed as `tests/testdata/search_queries.txt`. Do not
invent the set.
For each query, run four configurations and record top-k URLs + latency + credits:
@@ -277,6 +303,8 @@ Plus the mechanical gates:
5. Partial results: one source 500 + two 200 → 200 with `warnings`.
6. Dixie busy → rerank fallback, still 200.
7. Cache hit/miss metrics on a repeated query inside the hour bucket.
8. `permit_restricted` caller: `403 search not permitted`, zero outbound (httptest
spies); a fleet caller with the same query still hits the fakes.
## 11. Code layout
@@ -297,14 +325,16 @@ Do not print the file. Tests use httptest upstreams, never live keys.
- Canonical URL table (fragment, case, default port, trailing slash).
- Fan-in: 3 fake sources, one timeout, one 500, one 200 → partial + warning.
- Tavily client: assert outbound JSON (`include_answer` false, no extract); drop
`answer` from inbound.
- Tavily client: assert outbound JSON (`include_answer` false, `search_depth` basic,
no extract); drop `answer` from inbound.
- Public-source dialer: poisoned private A record refused; TLS uses system roots.
- Brave/SearXNG response mapping fixtures (from PLAN's live shapes + Hermes providers).
- Dialer: configured loopback allowed; other loopback port refused; request-supplied
URL ignored.
- Rerank fallback when embed gate is full.
- Cache key: same norm/query/sources/k/hour → hit; next hour → miss.
- Daily cap: Nth Tavily call skipped.
- Restricted caller: 403, zero upstream requests, no cache access.
- `/v2/search` golden: `success` + `data.web`.
- Contract harness: `Firecrawl(...).search("contract", limit=3)` against sifttest
(search fixtures on the local HTTP server, `-test-allow-prefix` already exists).
@@ -324,7 +354,8 @@ hit, `title\nurl\nsnippet`, blank line between. `--json` = native envelope. Non-
exits match fetch (5 = sift unreachable).
`skill/SKILL.md`: `sift search` becomes the first step for web search; `web_search`
is the fallback when sift is unreachable. Still `sift fetch` for URLs.
is the fallback when sift is unreachable — **except** for `permit_restricted`
callers (403 from sift; do not fall through to Tavily). Still `sift fetch` for URLs.
## 14. Rollout
@@ -340,8 +371,8 @@ is the fallback when sift is unreachable. Still `sift fetch` for URLs.
## 15. Open questions for Kyle
1. §9 — do `permit_restricted` queries go to Brave/Tavily?
1. §9 — restricted search: **(a)** none, **(b)** reduced SearXNG engines, **(c)** same
as fleet. Code fails closed at 403 until this is picked.
2. Tavily `daily_credit_cap` default (500 is a guess).
3. Brave LLM Context vs leaving Brave entirely inside SearXNG: bake-off should answer;
confirm we should not also call Brave Web Search.
)
+53 -3
View File
@@ -501,14 +501,14 @@ func (s *Server) writeFetchErr(w http.ResponseWriter, c *auth.Caller, rawURL str
fe.Msg = "internal error"
}
if fe.Status == 401 || fe.Status == 403 {
s.Log.Warn("refused", "caller", name, "host", host, "reason", fe.Msg)
s.Log.Warn("refused", "caller", name, "host", host, "reason", s.logReason(c, fe))
} else {
s.Log.Warn("fetch failed", "caller", name, "host", host, "reason", fe.Msg, "status", fe.Status)
s.Log.Warn("fetch failed", "caller", name, "host", host, "reason", s.logReason(c, fe), "status", fe.Status)
}
writeErr(w, fe.Status, fe.Msg, fe.Hint)
return
}
s.Log.Error("internal", "caller", name, "host", host, "err", err)
s.Log.Error("internal", "caller", name, "host", host, "err", s.logErr(c, err))
writeErr(w, 500, "internal error", "")
}
@@ -591,6 +591,56 @@ func (s *Server) logHost(c *auth.Caller, raw string) string {
return hostOf(raw)
}
func (s *Server) restrictedLog(c *auth.Caller) bool {
return c != nil && permittedCaller(c.Name, s.PermitRestricted)
}
// logReason is the reason field for fetch error logs. Restricted callers get a
// coarse class so the hostname in guard messages does not land in the journal.
func (s *Server) logReason(c *auth.Caller, fe *fetch.Error) string {
if fe == nil {
return "-"
}
if s.restrictedLog(c) {
return coarseReason(fe)
}
return fe.Msg
}
func (s *Server) logErr(c *auth.Caller, err error) any {
if s.restrictedLog(c) {
return "-"
}
return err
}
func coarseReason(fe *fetch.Error) string {
switch fe.Status {
case 401:
return "unauthorized"
case 403:
return "blocked"
case 400:
return "bad request"
case 413:
return "too large"
case 415:
return "unsupported"
case 429:
return "busy"
case 504:
return "timeout"
default:
if fe.Status >= 400 && fe.Status < 500 {
return "upstream 4xx"
}
if fe.Status >= 500 {
return "upstream 5xx"
}
return "-"
}
}
func writeErr(w http.ResponseWriter, status int, msg, hint string) {
if status == 502 {
status = 500
+31 -7
View File
@@ -284,7 +284,7 @@ func recHost(r slog.Record) string {
var host string
r.Attrs(func(a slog.Attr) bool {
if a.Key == "host" {
host = fmtString(a.Value)
host = a.Value.String()
}
return true
})
@@ -293,11 +293,17 @@ func recHost(r slog.Record) string {
func recMsg(r slog.Record) string { return r.Message }
func fmtString(v slog.Value) string {
if v.Kind() == slog.KindString {
return v.String()
}
return v.Any().(string)
func recDump(r slog.Record) string {
var b strings.Builder
b.WriteString(r.Message)
r.Attrs(func(a slog.Attr) bool {
b.WriteByte(' ')
b.WriteString(a.Key)
b.WriteByte('=')
b.WriteString(a.Value.String())
return true
})
return b.String()
}
func TestRestrictedCallerLogsBlankHost(t *testing.T) {
@@ -345,7 +351,7 @@ func TestRestrictedCallerLogsBlankHost(t *testing.T) {
src, _ := env["source"].(map[string]any)
final, _ := src["final_url"].(string)
if !strings.Contains(final, hostOf(ts.URL)) {
t.Fatalf("final_url still in body, got %q", final)
t.Fatalf("final_url missing from body, got %q", final)
}
rec = postJSON(t, s, "/v1/fetch", strings.TrimSpace(string(gsKey)), map[string]any{"url": "http://10.0.0.5/", "mode": "clean"})
@@ -353,6 +359,12 @@ func TestRestrictedCallerLogsBlankHost(t *testing.T) {
t.Fatalf("gs refused: %d %s", rec.Code, rec.Body.String())
}
const badHost = "this-host-does-not-exist-sift-p25.invalid"
rec = postJSON(t, s, "/v1/fetch", strings.TrimSpace(string(gsKey)), map[string]any{"url": "http://" + badHost + "/", "mode": "clean"})
if rec.Code != 403 {
t.Fatalf("gs nxdomain: %d %s", rec.Code, rec.Body.String())
}
rec = postJSON(t, s, "/v1/fetch", strings.TrimSpace(string(fleetKey)), map[string]any{"url": ts.URL, "mode": "clean"})
if rec.Code != 200 {
t.Fatalf("fleet fetch: %d %s", rec.Code, rec.Body.String())
@@ -386,6 +398,18 @@ func TestRestrictedCallerLogsBlankHost(t *testing.T) {
if fleetFetch != wantHost {
t.Fatalf("fleet fetch host=%q want %q", fleetFetch, wantHost)
}
h.mu.Lock()
for _, r := range h.recs {
if attrCaller(r) != "straylight-gs" {
continue
}
dump := recDump(r)
if strings.Contains(dump, badHost) || strings.Contains(dump, "10.0.0.5") {
t.Fatalf("restricted log leaked host: %s", dump)
}
}
h.mu.Unlock()
}
func attrCaller(r slog.Record) string {