17a2fda93929b3146de9bcb574f14d13befed476
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 jcoffey-dev
|
5365c92ffa |
Validate panels in Store.AddPanel/UpdatePanel, not just at the handler
CreateDashboard's inline panel-creation path already called validatePanel before insert; AddPanel and UpdatePanel relied on the HTTP handler to validate first instead of enforcing it themselves. Found via a live Postgres integration test: calling store.AddPanel directly (bypassing the handler) hit a viz_config NOT NULL constraint violation instead of getting the same default-empty-JSON treatment every other panel-creation path gets. |
||
|
jcoffey-dev
|
abeee0076b |
Build and browser-verify the tenant-picker frontend page
web/src/routes/select-tenant now calls enterprise-auth's existing
GET /auth/memberships / POST /auth/select-tenant protocol (built earlier
this phase, previously called only from Go tests) via
fetch(..., {credentials: 'include'}) -- new listMemberships/selectTenant
functions in $lib/api.ts, using a dedicated request helper that reads
plain-text error bodies (loginhandler's http.Error responses), unlike
every other request helper in that file which expects JSON.
Credentialed cross-origin fetch needed CORS enterprise-auth didn't have:
api/httpserver.WithCORS's wildcard-friendly default can't be combined
with a credentialed request at all (browsers refuse to honor
Access-Control-Allow-Origin: "*" on one) -- added WithCredentialedCORS
(literal origin, Access-Control-Allow-Credentials: true) alongside it,
wired into enterprise-auth via a new CORS_ALLOWED_ORIGIN config var
defaulting to POST_LOGIN_REDIRECT_URL (web's own origin, the same
default pattern SELECT_TENANT_REDIRECT_URL already used).
adapter-static's route crawler doesn't discover a page nothing links to
(this one is only ever reached via enterprise-auth's redirect) -- fixed
with select-tenant/+page.ts's `export const prerender = true`, the same
declaration every other route already has.
Genuinely verified in a real browser in this environment, not just
type-checked: a throwaway Node server standing in for enterprise-auth's
exact wire contract (including its plain-text error bodies), driven
through the full flow via mcp__claude-in-chrome -- cross-origin
pending-login cookie set, credentialed preflight + GET/POST round trip,
a real click choosing a tenant, the post-selection redirect, and the
missing/expired-cookie error path rendering the backend's actual
message. No Docker or live Postgres/IdP needed, since the point was
exercising web's own fetch/CORS/cookie wiring, not enterprise-auth's
internals (already covered by loginhandler's own tests).
This closes the tenant-picker as the last named gap in Phase 4. What's
left is the already-disclosed live-verification caveat shared by every
Postgres/ClickHouse-backed piece and both SSO protocols: none of this
has run against a real database, external IdP, or multi-container
deployment in this environment.
|
||
|
jcoffey-dev
|
2e698f5623 |
Close the last tenant-isolation adversarial probe (mid-provisioning tenants)
Phase 4 task 8's verification plan named four adversarial probes; three were closed earlier this phase, the fourth (an evaluator tick, or any other caller, hitting a tenant that exists but hasn't reached the active+credentialed gate yet -- must be refused, not served) was still an explicitly-skipped stub in api/queryapi/tenant_isolation_gap_test.go. Investigating it found the two storage engines needed genuinely different treatment: - ClickHouse (enterprise/internal/chrunner) already had this property structurally, for free: Registry is built once at startup from rbacstore.ListProvisionedDataSources, which already filters to active+credentialed tenants only, so a mid-provisioning tenant is simply absent from the connection map. New test TestRegistryRefusesMidProvisioningTenant proves this without Docker -- an empty DataSource list never dials ClickHouse, so this genuinely runs in this environment, unlike every other test in that file. - Tantivy (search/src/registry.rs's IndexRegistry) was a real, different gap, not just an unverified assumption: it opens-or-creates an index for any syntactically-valid tenant_id on first request, because it's a separate process with no Postgres access and structurally can't know which tenants are actually provisioned. A query against a mid-provisioning tenant would have silently succeeded with zero results from a freshly-created empty index -- "ambient success" indistinguishable from "no matching logs," exactly the failure mode this item was worried about. Fixed the Tantivy gap with a new enterprise/internal/searchclient. TenantChecker interface (backed by a new rbacstore.TenantIsActive, implemented structurally, no new import edge needed), consulted before every gRPC call: Client.Search now refuses a non-active tenant before it ever reaches `search`. Dial's signature gained a required TenantChecker parameter; enterprise-api's main.go passes its existing rbacstore.Store (already satisfies the interface). Verified Docker-free via searchclient's existing real-in-process-gRPC-server test harness (TestSearchRefusesMidProvisioningTenant, plus TestSearchPropagatesTenantCheckerError for the fail-closed-on-error case) -- both genuinely run in this environment, same bar as the rest of the Tantivy isolation work. rbacstore.TenantIsActive itself has two new skip-gated live-Postgres tests (TestTenantIsActive, TestTenantIsActiveNonexistentTenant) -- disclosed as not run against a live database here, same gap as the rest of this phase's Postgres-backed pieces. api/queryapi/tenant_isolation_gap_test.go rewritten from a checklist with one skipped stub to a full accounting of all four now-closed probes. Docs updated in lockstep: CLAUDE.md, threat-model.md, phase-4-isolation-design.md (implementation note added after its original sign-off), phase-4-runbook.md (§9), enterprise/README.md. |
||
|
jcoffey-dev
|
243f4dc2ab |
Enforce per-resource dashboard grants (RBAC matrix's own/granted qualifier)
api/dashboards' handler previously enforced only tenant-baseline role
(RoleEditor+), so any Editor could edit/delete any dashboard in their
tenant -- the matrix's "(own/granted)" qualifier was explicitly named
as unbuilt in this handler's own doc comment. This closes that gap.
New core interface api/dashboards.PermissionStore (nil-safe, same "not
wired == no-op" shape as authz.Authorizer) resolves a per-resource
dashboard_permissions grant. canEditDashboard now requires the
identity be Admin/Owner, the dashboard's creator, or hold a grant of at
least Editor; canManageGrants is deliberately stricter (creator or
Admin/Owner only, never grant-derived access) so a user who can edit a
dashboard only because of a grant can't extend or re-grant that access
to themselves or others. Wired handlers: PUT/DELETE
/dashboards/{id}/permissions/{userId}, GET .../permissions.
Two real bugs found and fixed while wiring this up, before any of it
touched a live database:
- handleCreate/handleImport never stamped created_by from the
authenticated identity, so every dashboard was owned by "anonymous"
regardless of who made it -- the ownership check would have been
meaningless. Also fixed: ImportDashboard trusted the exported JSON's
created_by verbatim, so re-importing someone else's export would
leave the actual importer unable to edit their own copy.
- metadata/migrations/0024_create_dashboard_permissions.sql's CHECK
constraint diverged from /docs/phase-4-rbac-design.md's schema
(allowed role='admin', nullable granted_by). Reconciled via
0033_restrict_dashboard_permissions_role.sql: Admin/Owner already
have tenant-wide access so a resource-level "admin" grant is
meaningless, and every real grant now always has an attributable
granter.
enterprise/internal/rbacstore gets the storage side: raw CRUD
(dashboard_permissions.go) plus DashboardPermissions
(dashboards_adapter.go), an adapter implementing
api/dashboards.PermissionStore -- same pattern as audit.QueryAPILogger
over queryapi.AuditLogger. Wired into enterprise/cmd/enterprise-api
only; plain api/cmd/api passes nil (ownership/Admin checks still work
via the nil-permissions fallback, just without the "granted" bonus).
Verified: the full own/granted/admin/creator matrix, including the
granted-editor-cannot-manage-grants regression, passes against a fake
PermissionStore (api/dashboards/handler_test.go, all existing tests
also still pass unmodified in behavior). Real integration tests exist
in enterprise/internal/rbacstore/rbacstore_test.go (skip-gated on
RBACSTORE_TEST_POSTGRES_ADDR, same convention as every other
Postgres-backed piece this phase) but have not run against a live
database in this environment -- disclosed in threat-model.md,
phase-4-runbook.md, and enterprise/README.md alongside every other
piece carrying the same gap. Also fixed a stale path in
phase-4-runbook.md's dashboards-tenant-scoping section
(./internal/dashboards/... -> ./dashboards/..., stale since that
package moved out of api/internal/ earlier in this phase).
|
||
|
jcoffey-dev
|
ba2276aa1a |
Phase 4: real Tantivy per-tenant isolation (search/src/registry.rs, enterprise/internal/searchclient)
Closes the last named "isolation mechanism" gap: search.proto gains a tenant_id field on SearchRequest; search/src/registry.rs's IndexRegistry resolves it to an on-demand-opened, per-tenant Tantivy index (empty tenant_id keeps today's single default index, so this is purely additive); enterprise/internal/searchclient sets that field from the authenticated request identity in ctx, mirroring chrunner's exact fail-closed "never a parameter" shape. Wired into enterprise-api in place of the shared api/searchclient. Unlike the ClickHouse pieces from the previous two commits, this one is genuinely verified end to end in this environment: Tantivy is an embedded library, not a networked service, so both the Rust index registry (cargo test, cargo clippy --all-targets -- -D warnings, both clean) and the Go client (a real in-process gRPC server) could actually run. registry.rs's tenant_index_is_isolated_from_default_and_other_tenants seeds three real indices with the same term and confirms a tenant-scoped search returns only that tenant's document -- item 3 of the isolation design doc's verification plan, closed for real, not just written. With both ClickHouse and Tantivy isolation now built, the single largest remaining gap is no longer a missing mechanism: it's that nothing forces or flags whether a deployment actually runs enterprise-api instead of plain api, and that ingest itself has no tenant concept for either storage engine (every record still lands in the one shared database/ index no matter what -- undesigned, not just unbuilt). Updated the threat model, architecture doc, CLAUDE.md, and both READMEs accordingly. |
||
|
jcoffey-dev
|
1d57e697b1 |
Phase 4: real per-tenant ClickHouse isolation via a new enterprise-api binary
Closes the threat model's headline finding for the SQL query path:
enterprise/internal/tenantprovision does real CREATE DATABASE/USER/GRANT
against ClickHouse, and enterprise/internal/chrunner is a per-tenant
connection registry implementing api's SQLRunner interface, resolving
the tenant from the authenticated request identity -- never a
caller-suppliable parameter. Both are wired into a new binary,
enterprise/cmd/enterprise-api, alongside the unchanged single-tenant
api/cmd/api, since AGPL core can never import enterprise/ and Go's own
internal/ package visibility rules meant enterprise/ couldn't implement
core's SQLRunner interface without importing the package that defines
it. That required moving api/internal/{authz,queryapi,dashboards,
querylang/executor,searchclient,httpserver} out of internal/ -- the
minimal set enterprise-api needs to import; querylang's compiler
internals (planner/lexer/parser/ast/ir) and api's own config stay
internal, since nothing outside api needs them directly.
Also finally wires enterprise/internal/audit into queryapi.AuditLogger
(nil since Phase 4 task 4) via a new adapter, and adds live-ClickHouse
integration tests for two of the four adversarial probes named in
docs/phase-4-isolation-design.md's verification plan.
Corrected several overclaims in the docs while writing this up: an
earlier claim that rbacstore's CRUD was "verified against a live
Postgres" was never actually true in this environment (only
internal/audit was, earlier in this phase, before Docker access was
lost) -- threat-model.md, phase-4-runbook.md, CLAUDE.md, and
enterprise/README.md all now distinguish "a real integration test
exists" from "this was confirmed against a live database."
Still not built: Tantivy/free-text tenant isolation
(enterprise/internal/searchclient), and any deployment-topology
mechanism that actually routes traffic to enterprise-api instead of
plain api -- both binaries exist side by side today with nothing
enforcing or flagging which one a deployment runs.
|
||
|
jcoffey-dev
|
3eb0f4c589 |
Phase 4: SSO scaffolding, RBAC enforcement, tenant-scoped dashboards, audit logging, K8s deployment
RBAC (api/internal/authz) is live on /query and /dashboards, backed by a new enterprise/ module (session issuance, audit logging, RBAC storage, OIDC/SAML protocol wiring) that core never imports -- only calls over HTTP. Found and fixed a real cross-tenant vulnerability in dashboards (no tenant_id filtering at all) while writing the threat model doc. Two things are explicitly NOT done, documented rather than hidden: tenant isolation for log data itself (/query still shares one ClickHouse connection and Tantivy index across every tenant -- RBAC controls who can query, not what a query can see), and human SSO login (protocol wiring exists, no HTTP handler calls it yet). See docs/security/threat-model.md and docs/phase-4-runbook.md. Also adds deploy/ (Go Operator + Helm chart, validated offline only -- no cluster was reachable in this environment). |
||
|
jcoffey-dev
|
9435115ab7 |
Phase 3: dashboards and alerting
Saved, shareable multi-panel dashboards (table/line/bar/single-stat panels via gridstack + uPlot, global + per-panel time range, JSON export/import) and threshold/absence alert rules with an ok/pending/firing evaluator and webhook/Slack/PagerDuty delivery. - New /metadata component: Postgres control-plane store for dashboards, panels, notification targets, alert rules/state, and delivery log -- see docs/phase-3-dashboard-design.md for why ClickHouse's MergeTree family isn't a fit for this access pattern (needs real row-level locking and read-your-writes consistency). - api/internal/dashboards: dashboard/panel CRUD, pure -- panel query execution stays client-side, reusing the existing /query endpoint. - New /alerting service: rule/target CRUD, a ticker-driven evaluator (claim-then-evaluate concurrency control, transactional-outbox delivery, query errors and threshold zero-rows never coerced into a false transition) and webhook/Slack/PagerDuty delivery with retry/backoff. See docs/phase-3-alerting-design.md for the full state-machine design and the four correctness properties it implements. - web: /dashboards and /alerts UIs; cli: sentryctl dashboards/alerts list/get/apply, seeding a future Terraform provider's JSON contract. - hack/alert-load-test: 500 rules against real ClickHouse data, real measured results in docs/phase-3-runbook.md. Five real bugs found by actually running this against a live stack (documented in the runbook, not just fixed silently): a latent Phase 2 bug where ClickHouse rejected the timestamp format used for earliest=/latest= queries; a "now" literal token injected into query text; a GridStack/uPlot layout-timing race; JS's Date.parse being too lenient to use as a timestamp-detection heuristic; a rule's "enabled" field silently defaulting to false when omitted; and the evaluator's claim-batch-size and worker-pool-concurrency defaulting to the same value, causing 500 concurrently-due rules to take 125s to cycle through instead of the configured 60s. |
||
|
jcoffey-dev
|
fb5049a747 |
Phase 2: unified query language spanning ClickHouse and Tantivy
Replaces the separate SQL-only /query and text-only /search endpoints with one pipe-syntax query language (plus raw SQL escape hatch) that compiles to a single IR and execution plan across both backends, so a query like `message:"connection refused" | stats count by host` runs as one request instead of two disjoint tools. - api/internal/querylang: lexer -> ast -> parser -> ir -> planner -> executor, each layer independently tested. - Execution generalizes Phase 1's proven Tantivy-prefilter pattern into a 4-way routing table (pure ClickHouse / text-only / text + aggregation / raw SQL passthrough). - Unified web query page and `sentryctl query`, both hitting the same POST /query endpoint. - Benchmarked against a real 1,022,000-row dataset (hack/benchmark-fixture); caught and fixed a real bug where the Tantivy prefilter cap (10,000) produced an IN-clause exceeding ClickHouse's default max_query_size -- lowered to 5,000, documented in docs/query-language-design.md and docs/phase-2-runbook.md. - docs/query-language-reference.md: customer-facing syntax reference. |
||
|
jcoffey-dev
|
cd8aa290ca |
Phase 1: Windows log collection + full-text search
Extends the agent, ingest, storage, api, and web with Windows Event Log/ETW sourcing and Tantivy-backed free-text search, per the approved Phase 1 plan. - CLAUDE.md: materialized on disk (never existed as a file before) with a new Phase 1 "done looks like" section. - agent: Windows Event Log (EvtSubscribe) and ETW sources, Windows service wrapper (install/uninstall/run-service), both feature- and target_os-gated so Linux builds/tests/clippy stay unaffected. Also fixed two pre-existing Phase 0 clippy gaps (dead-code on default-features-only builds, a type-inference edge case) found while testing every feature combination properly for the first time. UNVERIFIED on real Windows -- no Windows toolchain existed anywhere in the build environment; flagged prominently in three places. - proto/ingest: new record_id field, assigned once server-side in ingest's gRPC front end so ClickHouse and Tantivy agree on the same ID for the same record. - storage: record_id column + bloom filter index, verified against a live ClickHouse. - search: new service, Tantivy index, rskafka consumer as an independent second consumer group on the same Redpanda topic ingest already reads. - api/web: new /search endpoint and page, sharing the query page's result-table shape and component. - hack/windows-fixture: sends realistic Windows-shaped data straight to ingest, so the pipeline's handling of it is verifiable without a Windows host. Verified end-to-end on the live docker-compose stack: the same record_id comes back from both /query and /search for the same log line, including for windows-fixture's synthetic Windows Event Log data. Real bugs found and fixed along the way: api/Dockerfile missing proto/ in its build context, search's logs being completely silent (RUST_LOG gap), and search/target/ missing from .gitignore/.dockerignore. |
||
|
jcoffey-dev
|
b6b092c912 |
Scaffold Phase 0: agent -> Redpanda -> ingest -> ClickHouse -> api -> web
End-to-end log pipeline for Linux hosts, per /docs/architecture.md: - proto: shared gRPC contract (agent <-> ingest), Go bindings checked in - agent: Rust, musl-targeted, journald/file sourcing, RFC5424 parser, mTLS gRPC client, no required config for the common case - ingest: Go, single binary with --mode server|consumer|all; gRPC front end forwards to Redpanda unchanged, consumer normalizes and batch-writes to ClickHouse with at-least-once delivery - storage: ClickHouse schema + a plain SQL-file migration runner - api: minimal SELECT-only query endpoint, plain REST (not gRPC+gateway yet -- see api/README.md) - web: SvelteKit static SPA, one query page - transport: Redpanda compose + topic provisioning - cli: sentryctl ping stub - hack/dev-certs: throwaway CA + cert generation for local mTLS - root docker-compose.yml + docs/phase-0-runbook.md tie it together Not yet run end-to-end against real Docker/ClickHouse/Redpanda -- see the runbook's caveats section before relying on this working as-is. |