wizardchen 8efe1b4187 refactor(rbac): address kb-access guard review findings ...

Follow-up to the route-level KB-access guard introduced earlier in
this PR. Five reviewer findings, all fixed in middleware + tests
without changing handler call sites:

  - Honour `?agent_id` in the agent-share fallback. The in-handler
    resolution in knowledgebase.go branches on a specific agent's
    KBSelectionMode (all / selected / none); the guard was only ever
    calling TenantCanAccessKBViaSomeSharedAgent, which let some
    requests pass at the guard layer that the handler would then
    reject — and rewrote the request's tenant context to the source
    tenant before the rejection happened. The guard now mirrors the
    handler: a specific agent_id pins the resolution to that agent
    (no fallback to any-agent), an empty agent_id keeps the existing
    any-agent behaviour. The knowledge.go / knowledgebase.go handler
    helpers stay in place as defence in depth until they're folded
    into the guard in a follow-up.

  - Distinguish not-found vs. transient on the knowledge / chunk
    resolvers. A DB hiccup used to surface as a 404, both confusing
    clients and hiding the underlying failure from monitoring. The
    resolvers now return apperror 404 only for the known not-found
    sentinels (apprepo.ErrKnowledgeBaseNotFound, ErrResourceNotFound)
    and let other errors propagate; the guard maps those to a fresh
    NewServiceUnavailableError (503).

  - Stop 500-ing on legacy chunks with empty knowledge_base_id. The
    chunk is effectively unresolvable to a KB, so the client gets the
    same 404 they'd get for a missing chunk instead of a 500 that
    pollutes alerting. The warn log preserves the operator signal.

  - Honour cfg.Tenant.EnableRBAC. The guard now mirrors RequireRole /
    RequireOwnershipOrRole: with enforcement off, 401/403 paths log
    the would-be rejection and pass through. 404 still fires either
    way (a missing resource is not an authorisation event). Keeps the
    rollout window safe — operators can flip enforcement on globally
    without code changes elsewhere.

  - Document the c.Keys vs. c.Request.Context() split. The guard
    intentionally rewrites only the request context (handlers that
    still run their own share resolution, currently knowledge.go /
    knowledgebase.go, would otherwise see kb.TenantID == c.Keys
    tenant and misclassify shared-Viewer access as Admin). The
    package-level comment now spells this out so the migration of the
    remaining handlers in a follow-up doesn't accidentally regress
    `my_permission` rendering.

Tests:

  - Existing four cases keep working; runGuard signature gained a
    guardOpts knob.
  - Eight new cases cover the agent-share resolution paths (any-agent
    viewer, Editor required → must reject, specific agent in modes
    all / selected-match / selected-miss / none, cross-tenant
    mismatch) plus the EnableRBAC=false fail-open + 404-still-fires
    pair.

2026-05-18 17:28:58 +08:00

..

errors.go

refactor(rbac): address kb-access guard review findings

2026-05-18 17:28:58 +08:00

session.go

init commit

2025-08-05 15:08:07 +08:00