Release v0.5.0: api develop → main - #415
Merged
Merged
Conversation
…fail a migration (#155)
GET /api/v1/scans/coverage?window_days=30 (JWT; scans:read) returns a tenant-scoped rolling-coverage summary so RFC-007 scans become verifiable: total scannable, never-scanned, covered-in-window, stale, critical-never-scanned, critical-uncovered, oldest-dispatched, coverage_percent. - scancoverage.CoverageStats + CoverageStatsReader interface - ScanCoverageRepository.CoverageStats: one conditional-aggregation query over the scannable estate LEFT JOIN scan_coverage_state (tenant-scoped; SQL validated by PREPARE on PG17) - ScanHandler.CoverageStatus (window_days bound 1..3650; nil-reader + missing-tenant guarded) wired via repos.ScanCoverage; route under existing /scans group - handler unit tests (default/custom/invalid window, missing tenant, nil reader) - docs: scan-coverage.md Phase 4 section + roadmap The headline risk metric is critical_never_scanned. Capped-engine (.sc) license utilisation lands with Phase 3.5 accounting. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…epth) (#157) Update/Delete/TestIntegration fetched via GetByID then verified tenant ownership in the service (fetch-then-check). The audit flagged this foot-gun: the repo method is tenant-agnostic, so a future caller could forget the check. Add integration.Repository.GetByTenantAndID (tenant predicate enforced in SQL, returns ErrIntegrationNotFound for a missing OR other-tenant record) and use it on the three mutating service paths — moving the guarantee into the data layer, no fetch-then-check window. Behaviour is unchanged for valid callers. Tests: cross-tenant Update/Delete -> NotFound; the record survives a cross-tenant delete attempt. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
A notification that exhausts its retries is marked 'dead', then archived to notification_events and removed from the outbox — but it was logged only at Debug, indistinguishable from a successful send, so a permanently-failed notification vanished silently (audit finding). Emit a structured ERROR (alertIfDeadLettered) with tenant/event_type/title/ retry_count/last_error before archiving, so ops can alert on level=error. No new infra. Unit-tested (errors only for 'dead'; silent for completed/pending/failed/ processing). Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…agrams (#159) * docs(rfc-008): native shift-left CI/CD code scanning (agent-first) Plan to make OpenCTEM's own agent best-in-class for CI shift-left (SAST/SCA/ secrets + PR decoration + risk-aware gate), learning from the califio code-secure study WITHOUT depending on it. Grounds the work in an audit showing our agent is already a peer/ahead, and phases the remaining polish (Phase 1 risk-aware gate already shipped as agent #27). Indexed in docs/rfcs/README.md. * docs(rfc-008): architecture doc with structure + dataflow + component diagrams Add docs/architecture/shift-left-ci-scanning.md (Mermaid: component structure, end-to-end PR-scan sequence, finding repo-vs-branch storage model + invariants, responsibilities, phase status, code map) and register it in docs/README.md. Satisfies the document-fully standard (RFC + architecture doc + index). --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…se 3) (#160) POST /api/v1/agent/ingest/baseline-diff (agent API-key auth): given a PR's current-scan fingerprints + the base/target branch, returns which are NEW (not already open on the base branch) vs pre-existing tech debt. Lets a PR gate / inline comments focus only on findings the PR introduces — the highest-value learnable from the code-secure ASPM study, built on our occurrence model. - vulnerability.FindingRepository.FingerprintsOpenOnBranch (occurrences JOIN findings, tenant+branch+status='open' scoped; SQL PREPARE-verified on PG17). - ingest.Service.BaselineDiff: resolve repo asset + base branch; unknown repo/ branch -> all new (no history). Pure partitionByBaseline helper, unit-tested. - handler BaselineDiff + route /agent/ingest/baseline-diff (sibling of /ingest/check). Tenant from authenticated agent. - updated all FindingRepository test mocks for the new interface method. - docs: architecture phase table + endpoint contract. Naming: 'baseline-diff' (clear, consistent with /ingest/check) + explicit new_fingerprints/pre_existing_fingerprints fields. Agent consumption (gate + comment filter) is the next step. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
Phase 3 is now complete across api #160 (baseline-diff endpoint), sdk-go v0.4.0 (#35 Client.BaselineDiff + handler NewFingerprints comment filter), and agent #28 (gate.FilterNewFindings + main.go baselineNewSet, fail-safe). Update RFC-008 + the architecture doc: status line, capability table, phase sections, sequence diagram, code map. Also reflect the other phases that were already shipped/present (2/4/5/7); only Phase 6 export remains partial. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
) * feat(pentest): XLSX export for campaign findings (RFC-008 Phase 6) Adds a real .xlsx export format to GET /pentest/campaigns/{id}/findings/export alongside the existing CSV/JSON (format=xlsx). Unlike CSV, XLSX keeps multi-line cells (steps, PoC) clean and avoids delimiter pitfalls; the header row is bold and frozen. Refactors the shared column order + per-finding row builder into pentest_export.go so CSV and XLSX stay in lockstep, and rewrites the CSV path on encoding/csv (was hand-assembled). Both spreadsheet formats run every cell through sanitizeCSVCell to defuse formula injection (=,+,-,@). Adds excelize/v2. Tested: row mapping, CSV BOM + sanitization, and a round-trip that re-opens the produced workbook and asserts header + sanitized cells. * docs(rfc-008): Phase 6 — CSV/XLSX export shipped; note scheduler gap Mark CSV+XLSX findings export done (api#162 + ui#159). Document the real remaining gap: report_schedules + ListDue() + the report:generate_scheduled task exist but no controller invokes ListDue(), so configured schedules never run — wiring needs a generic report generator + auto-email cron (deferred, needs a product decision). --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
* docs(rfc-006): detailed Phase 3 — bidirectional Jira status sync Per a user use case (create task in OpenCTEM ↔ Jira, and Jira board status drag syncs back). Grounds the current state (outbound-create + inbound-status already work; outbound-status missing — client has no transition call) and specifies the missing half + the machinery a two-way loop needs: - provider GetTransitions/DoTransition/AddComment (Jira has no 'set status') - ticket_links typed table (replaces URL-substring heuristic; holds echo-guard bookkeeping: last_pushed/last_inbound status+time) - echo-guard: state-compare (skip-on-equal / skip-on-last-pushed) + origin tag - outbound delivery via the transactional outbox + bounded worker (retry/rate- limit/per-tenant fairness); opt-in per integration, default off - conflict policy (last-writer-wins by event time; FP/risk-accepted authoritative) - a WorkItem seam so the same engine serves findings now and a grouping remediation_task later (user wanted both) Sub-phases 3a (transitions) / 3b (ticket_links) / 3c (echo-guard+outbound) / 3d (per-tenant maps) / 3e (remediation_task entity). Links from parent RFC-006 and the RFC index. * docs(rfc-006): time-bound the echo-guard inbound compare A bare 'skip inbound if status == last_pushed' suppresses a later LEGITIMATE re-set to the same status (push Done -> echo skipped; weeks later a human re-drags to Done -> matches stale last_pushed and is wrongly dropped). Bound the echo match to a short window after last_pushed_at (+ clear last_pushed once consumed); the provenance tag remains the primary loop-breaker, the windowed compare is the net. * docs(rfc-006): full status-model evaluation (§3.6.1) Evaluate the status maps against all 14 finding statuses + the transition graph + approval/verify rules. Document: the inbound/outbound default matrices, the 3 domain constraints (RequiresApproval, RequiresVerifyPermission, no graph skips), round-trip stability, and why FP/accepted are not inbound-mapped. Verdict: defaults now sufficient+correct for stock Jira; richness gap covered by per- integration overrides + comment-fallback + approval-gating. --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
) Foundation for outbound status sync. Jira has no 'set status' — you POST a workflow transition whose availability depends on current status. Add: - GetTransitions(issueKey) — list available transitions (id, name, to-status) - DoTransition(issueKey, transitionID, comment) — perform one (+ optional comment) - AddComment(issueKey, body) — fallback when no transition reaches the target - TransitionToStatus(issueKey, targetStatus, comment) — resolve target status name → transition id (case-insensitive) and perform; ErrNoMatchingTransition when the workflow forbids the move (caller falls back to AddComment) No caller yet → zero behavior change; design-independent of the rest of RFC-006 Phase 3. httptest-covered (parse, match+post, no-match sentinel, comment, error). REST shapes per Jira v2; verify against a live appliance before enabling sync. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…FC-006 Phase 3d) (#168) Evaluated the maps against the full 14-status finding vocabulary + the domain transition graph + approval/verify rules. Changes: Outbound (NEW — finding status -> Jira status name): - StatusOutbound map + JiraStatusForFinding() + SyncEnabled (default OFF). - Stock-Jira defaults (To Do/In Progress/Done) covering new/confirmed/in_progress/ remediation/retest/fix_applied/resolved/verified. - Deliberately UNMAPPED (no stock status -> comment-fallback / customer config): false_positive, accepted, accepted_risk, draft, in_review, duplicate. Inbound (completeness + correctness): - 'open' -> confirmed (Jira's initial/unstarted status; was wrongly in_progress). - add 'duplicate' -> duplicate (webhook-settable, no approval), 'verified'/'reviewing'/'selected'. - Documented WHY false_positive/accepted are NOT inbound-mapped (RequiresApproval) and why every done-like status -> fix_applied not resolved (resolved needs verify permission; rescan hook promotes). No caller yet -> zero behavior change. config.ticketing gains status_outbound + sync_enabled. Tests cover outbound defaults, overlay, invalid-key skip, switch. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
Dependabot was opening PRs against main (the default branch). main can lag develop, so bumps failed CI on code develop had already fixed — e.g. a go-chi bump flags chimw.RealIP as deprecated (SA1019), which develop already removed but main still calls. The repo's workflow is 'PRs target develop'; align dependabot so its PRs are based on the clean integration branch and merge the normal way. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…3c) (#171) * feat(jira): outbound status-sync engine SyncFindingStatusToTicket (RFC-006 Phase 3c core) The outbound half of bidirectional sync: push a finding's status to its linked Jira issue. Self-contained + tested; no caller yet (activation = the finding status-change trigger + async wiring, a focused follow-up), mirroring how 3a (client transitions) landed as foundation. - Extend app-layer Client interface with GetIssueStatus/TransitionToStatus/ AddComment; clientAdapter forwards them and maps infra ErrNoMatchingTransition to the app sentinel so the caller can fall back to a comment. - SyncFindingStatusToTicket(tenantID, findingID, mapping): * opt-in gate (mapping.SyncEnabled, default off) — no surprise Jira writes; * resolves target via the merged status_outbound map; unmapped status = no-op; * issue key parsed from finding WorkItemURIs (firstJiraIssueKey); * ECHO-SAFE: only the OpenCTEM-initiated path calls this (the inbound webhook updates findings directly, bypassing it) + skips when Jira already at target; * no workflow transition to target -> comment fallback (never hard-fail). Tests: transition-when-enabled, disabled-noop, already-at-target skip, comment fallback on no-transition, unlinked-noop, issue-key parser. go build ./... + vet green (GOWORK=off). * feat(jira): activate outbound status sync (RFC-006 Phase 3c) Wire the outbound engine so it actually fires (still opt-in, default off): - MappingResolver (app) + IntegrationClientResolver.ResolveMapping (infra) load per-tenant status_outbound + sync_enabled from the integration config. - SyncService.SyncFindingStatus(tenantID, findingID) = async entrypoint: resolve mapping → SyncFindingStatusToTicket; no integration → no-op. - asynq task jira:sync_finding_status + JiraSyncTaskHandler (+ worker.go WithJiraStatusSyncer registration) + Client.EnqueueJiraSyncFindingStatus. - VulnerabilityService.SetJiraStatusSyncHook + trigger in UpdateFindingStatus: fires ONLY when status changed AND the finding has a work-item link (avoids noise). Echo-safe: the inbound webhook updates findings via a different path, so it never re-triggers outbound. - Wired in cmd/server: SetMappingResolver on JiraSync; NewJobWorker gets the syncer; main.go sets the enqueue hook from the job client. End-to-end now: OpenCTEM status change → enqueue → worker → resolve mapping/ client → transition Jira (or comment fallback), gated by config.ticketing. sync_enabled. Tests: SyncFindingStatus resolver paths + asynq handler (happy + bad-payload). go build ./... + vet + finding/jira/jobs tests green. --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…eting reference (#172) Update the ticketing architecture doc now that outbound status sync shipped (#167/#168/#171 + ui#170): both-ways overview, an Outbound status sync section (asynq flow + echo-safety + why-asynq-not-outbox), a full config.ticketing reference table (sync_enabled/status_outbound/status_inbound/...), corrected default mapping tables, roadmap (Phases 2&3 Done), and key files. Completes the 'document features fully' requirement for the bidirectional sync. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…elimiter panic guard) (#173) Three real bugs found by an adversarial cross-codebase review (the rest of the candidates were verified non-issues — webhook HMAC exists, github split is guarded, the 'fail-open' baseline path actually fails closed, switch-team is a Next route): 1. jira/client.go GetIssueStatus: issueKey was not url.PathEscape'd (every sibling method escapes it) → a key with URL-special chars would corrupt the echo-guard status check. Renamed the shadowing 'url' var to 'u' and escape. 2. finding bulk status: BulkUpdateFindingsStatus never fired the outbound Jira sync hook, so bulk status changes silently skipped Jira (single-finding did sync) — asymmetric. Now fires for each updated finding with a ticket link, mirroring the single path. Regression test added. 3. redis/ratelimiter: Allow/Status/AllowN type-asserted result[0..2] from .Slice() with no length check → a short/malformed Redis Lua reply panics the limiter goroutine. Added len(result) < 3 guards (3 sites). Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
* fix(websocket): send-on-closed-channel panic race in Client SendMessage checked c.closed under c.mu, RELEASED the lock, then sent on c.send; Close() set closed and close(c.send) after releasing the same lock. Interleaving (check passes -> Close closes the channel -> send proceeds) panics with 'send on closed channel' and crashes the whole API process on a routine websocket disconnect under load. Fix: hold c.mu across the closed-check AND the (non-blocking) send, and perform close(c.send) under the same mutex — the pair can no longer interleave, and the select/default send cannot deadlock against Close. Regression test hammers SendMessage from 8 goroutines against a concurrent Close (50 rounds, -race clean) + double-Close idempotency. * fix(websocket): hub channel senders hang after shutdown, stalling graceful stop After Run exits (ctx cancelled at server shutdown), Broadcast / DeliverLocal / RegisterClient / UnregisterClient sent on channels nobody reads — an in-flight HTTP handler broadcasting a finding event, or a ReadPump/WritePump defer unregistering its client, blocked forever and stalled graceful shutdown until the hard timeout. Add hub.done (closed via defer when Run returns); every channel send selects on it: post-shutdown broadcasts are dropped (Debug log), registers close the client, unregisters no-op. Regression test: stop the hub, then call all four senders — must return promptly (was: permanent hang). --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
The only report generator today is pentest-campaign-specific. Add a generic, tenant-wide executive summary (pkg/report.GenerateSummaryHTML) built from vulnerability.FindingStats: total/open/resolved KPIs, severity breakdown with bars, and a reporting-window movement section (new vs resolved → net backlog trend). Self-contained printable HTML; all dynamic values escaped by html/template (XSS-safe, tested). Dependency-free (caller maps FindingStats → SummaryInput) so it can also back on-demand export. This is the content engine for the scheduled 'executive_summary' report — the next piece is the scheduler controller that runs report_schedules.ListDue and delivers it (the schedule table + ListDue + cron lib already exist; no controller invokes them today). Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
Whole-platform evaluation (strengths verified across 3 deepdive rounds + competitive study; gaps in the operator/management layer) and a value-ranked roadmap. Tier 1 (report scheduler / remediation campaigns / risk trending) all build on existing infra with no product unknowns; Tier 2 breadth; Tier 3 commercial foundation. Serves as the index for 'what to build next and why'. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…ew-vs-resolved trend (#187)
…duled (#384) Creating an agent through the API without naming max_concurrent_jobs produces an agent that registers, heartbeats, reports healthy and shows online — and is never given a single job. Triggering a scan answers "No tenant agent available" while that agent sits there with the right tool. FindAvailableWithCapacity selects on `current_jobs < max_concurrent_jobs`, so a capacity of 0 fails on every pass. Nothing errors, because nothing goes wrong: the row simply never matches. The database already had the right answer. agents.max_concurrent_jobs carries DEFAULT 5 — but the column default is never reached, because the repository writes the field explicitly on INSERT. So the Go layer's zero overrides the schema's own sensible value instead of falling back to it. NewAgent never set it, and CreateAgent only assigns when the caller passes a positive number (service.go:121), so the zero survives all the way to the row. NewAgent now sets DefaultMaxConcurrentJobs = 5, chosen to equal the column default so the two layers stop disagreeing. SetMaxConcurrentJobs refuses non-positive values: the HTTP layer already validates min=1, and the domain should not depend on a caller upstream getting that right when the failure mode is silent. CLAUDE.md already lists this exact shape under "Handler defaults vs Service defaults" with the fix spelled out. The rule existed; the code did not follow it. Severity, measured rather than assumed: 0 of 68 agents on the live database are affected — every one has a capacity, most at 5. This is a latent trap for anyone creating an agent through the API, not an active outage. Found by using the API the obvious way while proving the scan loop end to end against a real agent binary. Three of the four tests were confirmed to fail without the fix; the fourth is a no-regression check that an explicitly supplied capacity still wins. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…uced (#385) A real scan on a live stack pushed 2 findings. The agent logged "Push completed: 2 findings created", the database held 2 — and the pipeline run recorded total_findings = 4. OnStepCompleted calls stepRun.Complete(findingsCount), which stores the count on the step run. calculateRunStats then sums exactly those step runs. Adding findingsCount on top of that sum counts this step's findings a second time. The doubled number did not stay in one column. It also reached: - the audit event metadata for both the success and failure branches - the user-facing message, "Pipeline run completed successfully with N findings" so a run that found 2 secrets told the operator it found 4, and left that claim in the tamper-evident audit trail. OnStepFailed (run.go:580) always computed this correctly from calculateRunStats alone. The two paths disagreed and only the quieter one was right, which is part of why it survived: nothing compares them. Fix: drop the extra addition at all four sites so the run total is derived purely from the step runs, matching OnStepFailed. With multiple steps the old arithmetic compounded — every completion re-added the current step's count on top of a sum that already included it. Found by running an actual scan end to end against a real agent binary rather than by reading the code; the arithmetic looks reasonable until you have a real number to check it against. The regression test was confirmed to fail against the old expression. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…ed (#396) * fix(priority): deliver priority escalations — publisher was never wired PriorityClassificationService emits a PriorityChangeEvent on every class transition and nil-guards the publisher. No production implementation of PriorityChangePublisher existed and SetChangePublisher was never called in cmd/server/services.go, so publishIfChanged returned at its nil check on every call. A finding escalating from P3 to P0 — the CTEM signal an operator most needs — notified nobody. The PriorityFloodGuard that IS wired was guarding a fan-out that could not occur. Adds OutboxPriorityChangePublisher over the existing notification outbox (no new channel) and wires it. It filters to escalations only: first classification is already announced by new_finding and would double-notify every ingested finding, and a de-escalation does not need to interrupt anyone. Delivery severity is derived from the new class (P0 critical ... P3 low) so the tenant's existing per-integration severity filter routes it with no new knob. Registers finding_priority_escalated in AllEventTypes(). Without this the feature would still be inert: enabled_event_types is an opt-in whitelist, so an unregistered type matches zero integrations, takes the "no matching integrations" branch, is marked completed, archived and deleted — enqueued successfully and delivered nowhere, silently. Migration 000200 backfills existing rows; it deliberately skips EMPTY arrays, which are the legacy "allow all" state that appending to would have silenced. Guard test for the recurring class: cmd/server/wiring_guard_test.go resolves the Set* seams on PriorityClassificationService by reflection and asserts each is called on Services.PriorityClassification in cmd/server, or carries an explicit documented exemption. It matches the selector expression, not the bare method name, because cmd/server/handlers.go has an unrelated SetChangePublisher on CompensatingControlHandler that a name-only search would match. Verified red against the unwired code. TestDefaultEnabledEventTypes now asserts membership plus "every default is a real selectable type" instead of a bare count of 3. * fix(lint): replace deprecated parser.ParseDir in the wiring guard staticcheck SA1019: ParseDir is deprecated since Go 1.25. Enumerate the package's non-test files and ParseFile each instead, keeping the vacuous-pass guard (now on the parsed-file count). make lint-ci is green, which is what the Lint job actually gates on — lint-new alone was not enough to catch this. * fix(migrations): renumber to 000201 — #397 also added a 000200 Both this branch and #397 were cut from the same develop and both added a migration numbered 000200. Each PR is green on its own; the collision only exists after both merge, and golang-migrate keys on the numeric prefix — the second file never runs and `migrate up` errors. Renumbered to 000201 and updated the comment in notification_extension.go that names it. Also adds a duplicate-version check to scripts/check-migrations.sh, which already runs in the Migration Safety job. It scans the whole directory rather than the changed files, because that is the only way to see a collision that neither PR's own diff contains. Verified it fails on a planted duplicate and passes once removed. --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
… nobody (#397) Three defects in the platform-job half of the command lifecycle. Together they meant a scan routed to platform agents could hang indefinitely while the job-recovery controller logged zeroes and looked healthy. 1. recover_stuck_platform_jobs could never match a row. It required `platform_agent_id IS NOT NULL`. Nothing sets that column: get_next_platform_job is its only writer and has no Go caller, and Command.AssignToPlatformAgent has no callers at all. In practice a platform job is claimed by an ordinary *tenant* agent through GET /api/v1/agent/commands — GetPendingForAgent and ClaimForAgent do not filter on is_platform_job and platform jobs are created with agent_id NULL, so every tenant agent sees them — and ClaimForAgent sets agent_id instead. The gap that leaves is not cosmetic. A platform job acknowledged by a tenant agent that then dies was unreachable by every reaper: this function skipped it, recover_stuck_tenant_commands excludes it by design (is_platform_job = FALSE, 000172), the queue expiry only looked at 'pending', and fail_exhausted_commands needs dispatch_attempts >= max — which only the two recovery functions ever increment. The job sat in 'acknowledged' forever and its pipeline run waited on it. Migration 000200 matches the state that actually occurs, mirroring what 000172 did for tenant commands: increment dispatch_attempts so fail_exhausted_commands has a stopping condition to take over from. 2. RecoverStuckJobs accepted maxRetries and dropped it. The Go wrapper bound only $1; the SQL hardcoded `dispatch_attempts < 3`. Latent only for as long as the configured value stays 3. 3. ExpireOldPlatformJobs expired jobs without telling the owning run. It was a raw UPDATE in JobRecoveryController — the same mistake ExpireOldCommands made for tenant commands, one step over and worse. Platform jobs are created by scan/pipeline dispatch carrying pipeline_run_id + step_key and with expires_at NULL, so FindExpired never covered them and this UPDATE was the *only* thing that ever reaped them. Every job that timed out in the queue took its run down silently; the step stayed 'queued' until ScanTimeoutController reported a generic timeout instead of "expired in queue". Replaced by CommandRepository.FindQueueExpiredPlatformJobs, with expiry moved to app/command.ExpirationChecker — the component that already owns expiry precisely because it can call pipeline.OnStepFailed. Queue expiry now reports PLATFORM_JOB_EXPIRED_IN_QUEUE, distinct from COMMAND_EXPIRED. The migration keeps the one-argument signature as a delegating wrapper rather than dropping it, so old pods mid-rolling-deploy keep working (and pick up the corrected WHERE clause). Tests: DB-backed coverage of both recovery states and the queue-expiry boundaries, plus in-package tests asserting the run is notified with the right code. Each fails against the unfixed code. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
* fix(config): three settings an operator can set that did nothing
Each of these parsed an environment variable at boot and then had no
reader on any path an operator could observe.
1. Agent load-balancing weights (AGENT_LB_*). config.LoadBalancingConfig
parsed seven variables that nothing outside config.go read, and the
selector ranked agents purely on CurrentJobs/MaxConcurrentJobs. The
heartbeat path made it worse: it called UpdateMetrics, which never
touches LoadScore or MetricsUpdatedAt, so the load_score column never
reflected anything. Weights now flow config -> AgentService (persisted
score, recomputed each heartbeat) and config -> AgentSelector (job
placement). The disk/network throughput ceilings became weight fields
instead of file-scope constants, and the heartbeat payload accepts the
disk/network metrics those two weights need. Resource metrics older
than 5 minutes are ignored so one stale sample from a wedged agent
cannot bias scheduling forever.
2. Admin-audit-log retention. cmd/server/workers.go hardcoded
DryRun: true with no plumbing, so admin_audit_logs grew forever and
the 365-day policy could not be enforced by any deployment. Now
configurable via ADMIN_AUDIT_RETENTION_{ENABLED,DRY_RUN,DAYS,
INTERVAL,BATCH_SIZE}. DryRun still defaults to true: deleting audit
history on upgrade would be a compliance incident, and the controller
already reports what it would delete. That report is now WARN-level
with the variable to flip. Config validation refuses to boot with a
retention window under 30 days while deletion is enabled, because a
zero or negative window moves the cutoff to now-or-later and empties
the table.
3. AI_RATE_LIMIT_RPM and the auto-triage defaults. No limiter existed
anywhere, so a setting that reads as a spend cap enforced nothing. A
token-bucket limiter (golang.org/x/time/rate, already a direct
dependency) now sits in front of Provider.Complete, keyed per
credential so platform-mode tenants share the platform key's budget
and each BYOK tenant gets its own. Over-budget calls wait, then fail
with ErrRateLimited rather than reaching the provider. Separately,
AI_AUTO_TRIAGE_DEFAULT_{ENABLED,SEVERITIES} and AI_AUTO_TRIAGE_DELAY
now seed tenants that have never configured auto-triage; a tenant's
explicit choice, including an explicit off, still wins.
Behaviour is unchanged for a deployment that sets nothing: the shipped
weights match the previous constants, retention stays a dry run, the
rate limit default of 60 rpm is far above real triage volume, and
AI_AUTO_TRIAGE_DEFAULT_ENABLED defaults to false.
* fix(llm): scope the RPM budget by tenant identity, not by API-key digest
CodeQL go/weak-sensitive-data-hashing flagged the SHA-256 over the API
key that keyed the limiter registry. The alert is a heuristic — the
digest was a map key, never stored or compared for authentication — but
hashing a credential to partition rate limits was the wrong shape
regardless.
Scope now derives from AI mode plus tenant ID: platform-mode tenants
share one budget because they share the platform key's bill, and each
BYOK tenant gets its own. No credential is involved, the mapping is
readable in a log, and billing identity is the more accurate unit
anyway. CreateProvider keeps its signature and delegates to the new
CreateProviderForTenant.
* docs(llm): comments say budget scope, not credential scope
---------
Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
Bumps the go-minor-patch group with 6 updates: | Package | From | To | | --- | --- | --- | | [github.com/aws/aws-sdk-go-v2](https://github.com/aws/aws-sdk-go-v2) | `1.43.0` | `1.43.2` | | [github.com/aws/aws-sdk-go-v2/config](https://github.com/aws/aws-sdk-go-v2) | `1.32.31` | `1.32.33` | | [github.com/aws/aws-sdk-go-v2/credentials](https://github.com/aws/aws-sdk-go-v2) | `1.19.30` | `1.19.32` | | [github.com/aws/aws-sdk-go-v2/service/s3](https://github.com/aws/aws-sdk-go-v2) | `1.106.0` | `1.106.2` | | [github.com/aws/aws-sdk-go-v2/service/sts](https://github.com/aws/aws-sdk-go-v2) | `1.45.0` | `1.45.2` | | [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) | `5.19.1` | `5.19.2` | Updates `github.com/aws/aws-sdk-go-v2` from 1.43.0 to 1.43.2 - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@v1.43.0...v1.43.2) Updates `github.com/aws/aws-sdk-go-v2/config` from 1.32.31 to 1.32.33 - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@config/v1.32.31...config/v1.32.33) Updates `github.com/aws/aws-sdk-go-v2/credentials` from 1.19.30 to 1.19.32 - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@credentials/v1.19.30...credentials/v1.19.32) Updates `github.com/aws/aws-sdk-go-v2/service/s3` from 1.106.0 to 1.106.2 - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@service/s3/v1.106.0...service/s3/v1.106.2) Updates `github.com/aws/aws-sdk-go-v2/service/sts` from 1.45.0 to 1.45.2 - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@service/s3/v1.45.0...service/kms/v1.45.2) Updates `github.com/go-git/go-git/v5` from 5.19.1 to 5.19.2 - [Release notes](https://github.com/go-git/go-git/releases) - [Changelog](https://github.com/go-git/go-git/blob/main/HISTORY.md) - [Commits](go-git/go-git@v5.19.1...v5.19.2) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2 dependency-version: 1.43.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch - dependency-name: github.com/aws/aws-sdk-go-v2/config dependency-version: 1.32.33 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch - dependency-name: github.com/aws/aws-sdk-go-v2/credentials dependency-version: 1.19.32 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch - dependency-name: github.com/aws/aws-sdk-go-v2/service/s3 dependency-version: 1.106.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch - dependency-name: github.com/aws/aws-sdk-go-v2/service/sts dependency-version: 1.45.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch - dependency-name: github.com/go-git/go-git/v5 dependency-version: 5.19.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…399) integration_notification_extensions.enabled_event_types is an opt-in whitelist and ShouldNotifyEventType matches literal members of it. Six event types that production code enqueues were never registered as constants, so they matched zero integrations, took the "no matching integrations" branch in internal/app/outbox.Service, and were marked completed, archived and deleted — no error, no warning, no delivery. sla_breach was the worst of them: an SLA breach that notified nobody. Registers finding_assigned, sla_breach, approval_requested, approval_approved, approval_rejected and workflow_notification; adds the first four to DefaultEnabledEventTypes(); backfills existing non-empty whitelists in migration 000202 (an EMPTY array is the legacy "allow all" state and is deliberately left alone). Adds tests/unit/outbox_event_type_registry_test.go, which parses the tree and fails when an outbox event type is emitted but unregistered. Co-authored-by: Nguyen Manh <nvmanh66@gmail.com> Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
… clients copy it (#400) * fix(notifications): six event types were delivered nowhere, silently integration_notification_extensions.enabled_event_types is an opt-in whitelist and ShouldNotifyEventType matches literal members of it. Six event types that production code enqueues were never registered as constants, so they matched zero integrations, took the "no matching integrations" branch in internal/app/outbox.Service, and were marked completed, archived and deleted — no error, no warning, no delivery. sla_breach was the worst of them: an SLA breach that notified nobody. Registers finding_assigned, sla_breach, approval_requested, approval_approved, approval_rejected and workflow_notification; adds the first four to DefaultEnabledEventTypes(); backfills existing non-empty whitelists in migration 000202 (an EMPTY array is the legacy "allow all" state and is deliberately left alone). Adds tests/unit/outbox_event_type_registry_test.go, which parses the tree and fails when an outbox event type is emitted but unregistered. * feat(notifications): serve the event-type registry instead of letting clients copy it integration.AllEventTypes() is the registry the notification outbox routes on, and it had no HTTP consumer. Every client that needed the catalog kept a hand-written copy, and those copies drifted: six event types registered server-side had no way to reach an operator's screen. Adds GET /api/v1/me/event-types, tenant-scoped and module-filtered, carrying label, description, category and the default-enabled flag so a client renders the catalog without a second mapping of its own. Also removes two things that were already lying about this surface: - TenantModulesResponse.EventTypes was declared but never assigned by buildModulesResponse, so it never appeared on the wire. - The OpenAPI spec documented GET /event-types and GET /tenants/{slug}/event-types, neither of which is registered (both 404 against a running server), and gave /me/event-types a closed-source-era shape of id/slug/severity_applicable that no handler ever produced. Filtering is by the tenant's enabled modules, deliberately not by the caller's permissions. enabled_event_types is tenant-level configuration and nothing validates it on write, so hiding a type from one admin would let them erase it for everyone the next time they saved the channel. Category labels move into the domain alongside the types, for the same reason the types are there: the approval and workflow categories were added to the constant block without any client learning of them, and a category with no label renders as an untitled group with nothing failing at build time. --------- Co-authored-by: Nguyen Manh <nvmanh66@gmail.com> Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
Nothing compared the Go module catalog against the `modules` table.
TestPresetsReferenceKnownModules checks presets.go against CoreModuleIDs /
UserFacingModuleIDs / ModulePermissionMapping — code against code. The
migration seed was never an input to any test.
That matters because the database is the authority at runtime:
getTenantDisabledModules enumerates ModuleRepository.ListActiveModules()
(`WHERE is_active = TRUE`) and decides enablement from those rows. Add a
module in code, forget the migration row, and nothing fails — the module is
simply absent from the enumeration, so a tenant who subscribes to a bundle
referencing it can never have it enabled, gated or shown. Silently, at
runtime, in production. Migration 000187 records the same class landing the
other way round: `pipelines` lingered as a live gate over a catalog row that
had been superseded, and 403'd real routes for subscribed tenants.
Four checks against the migrated test database (CI applies migrations/ before
go test; skipped, as the other *_db_test.go files are, when DATABASE_URL is
unset):
1. every module a preset bundle enables — via ResolvePresetModules, so the
explicit allow-list plus core, mandatory and hard transitive deps — has
an ACTIVE row;
2. every module wired into ModuleGate.RequireModule has an ACTIVE row and
is not deprecated/disabled;
3. every ID in the three Go maps has a row (existence only — the maps
deliberately retain retired IDs such as sources/secrets/scope so
historic permission lookups resolve);
4. sub-module IDs and parent_module_id agree: a code-referenced
"<parent>.<child>" has its parent seeded, and every seeded sub-module row
points at the parent its ID names.
The gate list in (2) is parsed out of internal/ with go/ast, resolving
moduledom.ModuleX constants from pkg/domain/module — not hardcoded. Today
that derives exactly the seven gated modules; the eighth, added next month by
someone who never reads this file, is covered automatically. An unrecognized
call form is an error, not a skip, so the derivation cannot quietly lose
coverage.
Direction is asserted one way only. The table legitimately holds more rows
than the Go maps (88 active vs 57): UserFacingModuleIDs is sidebar-visible
modules only, excluding agents/tools/pipelines by design. DB ⊋ code is
expected; only code ⊆ DB is checked.
Guarded against vacuous passes, since every check is "for each X, assert ..."
and trivially true for an empty X: each derived set and the loaded table must
be non-empty, or the test fails naming what changed shape.
Failure messages name the missing ID, the preset/map/call site that
references it, and the INSERT to add — whoever trips this is adding a
feature, not studying the module system.
Parity is intact today; all four pass against a pristine fully-migrated
database. Verified red by adding a module ID consistently across
ModulePermissionMapping and presetCTEMFull with no migration row: every
pre-existing module test stayed green while the new guard failed naming it.
Also verified red for a newly gated module with no row, a gate on an active
but deprecated module, a sub-module with a NULL parent_module_id, an
unresolvable gate argument, an empty derived set, and an unmigrated table.
Also corrects a stale comment in presets_ctem_full_test.go: it described
deprecating the five legacy duplicate IDs as a pending cleanup item, but
migration 000187 shipped it.
Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…403) cmd/server/services.go called SetSafeCheckDispatcher(s.ValidationRun) fourteen lines before s.ValidationRun was assigned, so it stored a nil *validation.RunService. A nil pointer in a non-nil interface is the trap. tryDispatchLive guards with `s.safeCheck == nil`, which is FALSE for a typed-nil, so it walks past the guard and calls DispatchSimulationCheck on a nil receiver. That derefs s.assets and panics; the HTTP middleware turns it into a 500. So RFC-012 Phase 1b has been dead since it merged, and not quietly: any simulation carrying at least one target asset 500s on POST /simulations/{id}/run. One with no targets returns early and looks healthy, so it fails by data shape rather than uniformly. It was never seen because attack_simulations has no rows on any live tenant. Two changes. The wiring moves below the assignment, which is the actual fix. And the setter now rejects a nil — including a typed one — so the next mis-ordered call degrades to the synthetic fallback the code already documents, instead of crashing. The guard is what stops this recurring; the reorder alone would leave the same landmine for the next collaborator wired here. Three tests on the setter. The typed-nil case fails against the unguarded version with the exact symptom; the companion asserts a real dispatcher is still accepted, because a nil check that is too broad would silently disable live dispatch and look identical to today. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…ert (#404) Compensating controls had 0 rows on every tenant because the feature was broken end to end, in four independent places. 1. Create could never succeed. control_type/status/reduction_factor went straight into the INSERT unvalidated, so a value outside the CHECK constraints surfaced as a 500 with "internal error" instead of a 400 saying what was wrong. Validation now mirrors the CHECKs, sourced from the pkg/domain/compensatingcontrol constants (which already carried the correct vocabulary and had zero importers). 2. Even a VALID create returned 500. Create/Update/RecordTest scanned their own RETURNING clause into plain strings, but description, test_result, test_evidence and created_by are NULL on a fresh row — "converting NULL to string is unsupported", raised AFTER the INSERT committed. The caller saw a failure while the control really had been created. All five read sites now share one null-safe scanControl. 3. Compensating controls could not affect priority on the path that control changes actually drive. Linking an asset publishes a reclassify sweep (LinkAssets -> Reclassifier.reclassifyAsset -> ClassifyFinding), but ClassifyFinding never consulted the control lookup — only the batch and explain paths did. The fan-out built to make control changes move priority fed a classifier that could not see controls. The lookup is now applied through one shared helper used by all three paths. 4. RecordTest with test_result='fail' left status='active', so the API reported a failed control as active (scoring was saved only by the effective-control SQL separately excluding failures). It now deactivates, matching the domain rule. An empty/unknown test_result is a 400, not a 500. reduction_factor stays a 0-1 fraction on the wire — it is what the column (DECIMAL(3,2), CHECK 0..1) stores. A 0 factor is now rejected at create: the classifier only treats an asset as protected when the factor is > 0, so a control saved with the column default would have been a silent no-op. Note on scope: item 3 changes classification outcomes, but only for assets an operator has deliberately linked to an effective control. It applies the existing binary IsProtected rule on a path that was missing it; it does not make reduction_factor band the outcome. The factor remains presentational (a 0.05 and a 0.95 control both yield P2) — banding would change scoring for every tenant and needs its own change and its own argument. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…h time (#406) Releases here are squash-merged, so main's HEAD ends up with a single parent and git cannot see that develop already contains it. The next release then reports conflicting files that are not disagreements — 27 of them the first time, in files neither side had meaningfully diverged on. Merging main back into develop is the textbook fix and it does not stick: the following release squashes again. That has now happened four times, and each time the recovery was the same careful sequence done by hand. So this stops arguing with the merge strategy. The branch carries its own ancestry: develop's tree byte-for-byte plus one `merge -s ours` commit recording main as a parent. It merges cleanly whichever button is pressed, and it is one command. The part that matters most is the refusal. `-s ours` discards main's side entirely, which is only honest if develop already contains everything main has — so the script counts files present in main and absent from develop and exits rather than run when that is non-zero. If someone hotfixes main directly, that check is the only thing between the fix and oblivion. Verified against a real pair of refs where the count is 5, including a controller develop had deleted. Also refuses a malformed version, a branch that already exists (it will not force-push), and any state where the resulting tree does not match develop or main did not become an ancestor — it verifies rather than assumes, and does not push if either check fails. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…ined (#407) * feat(openapi): the spec is generated and CI-enforced, not hand-maintained api/openapi/swagger.yaml was generated by swag once and hand-drifted ever since. Nothing checked it, and it stopped describing this server: • 30 documented paths had no handler and no route anywhere in the repo — /admin/platform-agents, /admin/bootstrap-tokens, /plans, /tenants/{id}/subscription and friends, left over from a closed-source era. • 40 real endpoints were undocumented, including the entire /notifications API and GET /auth/providers. • 43 operations were annotated with the basePath baked in (@router /api/v1/groups under basePath /api/v1), so the spec advertised /api/v1/api/v1/groups. That covered all of /groups, /permission-sets, /credentials and /me/permissions. • 24 more named a path or method no router serves: /config/finding-sources (real: /finding-sources), /tool-stats (real: /tenant-tools/stats), /tenant-tools/bulk-disable (real: bulk/disable), PATCH /asset-groups/{id} (real: PUT), /agents/{id}/disable (real: /deactivate), /ingest/check (real: /agent/ingest/check), /assets/bulk-sync (real: /assets/bulk/sync). This is not a docs problem. The UI is written against this file, which is how it came to call GET /me/event-types — a path that never existed in OSS — and to render OAuth buttons for providers whose routes were never registered. Worst of the set: all five SecretStoreHandler operations claimed /credentials while the real group is /secret-store. The two GETs collided with the genuine leaked-credential endpoints, so the spec documented GET /credentials with the wrong request and response schema entirely. Changes: • Correct 65 @router annotations against the real route registrations, and one @Param (tool_id → toolId, which is what chi.URLParam actually reads). • Drop the two Keycloak-era annotations on AuthHandler: /auth/keycloak/info and /auth/keycloak/token are served at /auth/info and /auth/token, and LocalAuthHandler already annotates those paths. The handlers are mutually exclusive at runtime, so only one can own the annotation. • Regenerate the spec, and add scripts/check-openapi.sh + a CI job that fails when the committed file differs from a fresh regeneration. • make swagger used to print "swag not installed" and exit 0 — a regeneration that never ran looked identical to one that succeeded. It now installs the pinned swag and fails on error. The stray `echo "" >>` that appended a blank line the committed file never had is gone too. The gate is deliberately unconditional: no if: on event_name, no base-ref resolution, and it fails rather than skips when swag is unavailable. Two gates in this repo have already turned out never to run. Spec operations that name a path no route serves: 67 → 2. The two remaining are GET /health and GET /ready, which really are registered on the root router, outside basePath; Swagger 2.0 has no per-operation basePath, so the handler doc now records the caveat. Found, not fixed: IntegrationHandler.SendNotification is complete — it resolves the tenant, validates the body and calls the service — but is registered on no route, so POST /integrations/{id}/send 404s. Its @router was removed rather than left lying; wiring up a new authenticated write path that emits outbound traffic belongs in its own reviewed change. 459 registered routes are still undocumented. The gate freezes the debt rather than paying it down: annotations added from here on are verified, and the spec can no longer drift from the ones that exist. * fix(openapi): download modules before generating, or the spec silently degrades The gate failed on its own first CI run, with a diff of dozens of `format: int64` lines that CI's regeneration dropped. Cause: swag's --parseDependency reads dependency source to resolve types, and when a package is missing from the module cache it does not fail. It emits a degraded schema — plain `type: integer` where the real type is int64. Locally the cache is warm from every build and test, so the full spec is generated and committed; the CI job has no `go mod download`, so it regenerates a degraded one and the two disagree over lines describing identical types. For a generated-artifact gate that is the worst failure shape: it is not wrong about anything real, it just cannot be satisfied, and the obvious next move is to weaken it. `go mod download` now runs inside scripts/check-openapi.sh and the `swagger` make target, rather than in the workflow. Determinism belongs with the generator: `make swagger`, `make swagger-check` and CI must produce the same bytes no matter who invokes them, and putting it in the caller leaves the next caller to rediscover this. Verified: the gate passes on the committed spec, and still fails on a hand-edited one — renaming a single path makes it exit 1 and name that path. My first attempt at that check was worthless, since the anchor I edited did not exist in the file, so the "tampered" spec was byte-identical and both runs passed. * fix(openapi): swag needs a warm module cache to be reproducible The new gate failed on its first CI run, and it was right to. swag runs with --parseDependency, so it walks into dependency packages to resolve types declared outside this module. With a cold module cache it does not error — it silently emits a less-resolved schema: `format: int64` disappears from integers whose Go type comes from a dependency, and x-enum-descriptions vanishes for enums declared in one (the asset-type catalogue, 41 lines of it). So the generator produced one spec on a developer machine with a warm cache and a different one on a clean runner. A gate that only passes where the cache happens to be warm is the same silently-inert shape this PR is trying to close, just inverted. `go mod download` now runs before swag in both scripts/check-openapi.sh and `make swagger`. * fix(openapi): prove the dependency source is readable before generating The gate is still red in CI and the diff is the same 41 lines of x-enum-descriptions plus 12 `format: int64`. Both come from github.com/openctemio/ctis, which swag reads through --parseDependency: the asset-type catalogue's descriptions live on the ctis const block, not on this repo's. Without --parseDependency swag does not merely lose them, it fails outright (json.RawMessage becomes unresolvable), so the flag is not optional and neither is the dependency source. `go mod download` returning 0 turned out not to be proof that the source is extracted on disk. So assert it: go list -m -f '{{.Dir}}' for ctis and sdk-go must name a directory that exists, or the script stops with exit 2 instead of generating a degraded spec and blaming the committed file. Also switched to 'go mod download all' and stopped swallowing its output, and the failure message now prints the tail of swag's own log — 'cannot find type definition' there is the tell that the generator degraded rather than the spec being stale. * fix(openapi): assert the dependency graph loads, not a hardcoded module list The previous commit named ctis and sdk-go explicitly and got it wrong: sdk-go is not a dependency of this module, so 'go list -m' returned empty and the guard failed locally on a perfectly good tree. Ask the question swag actually asks instead — 'go list -deps ./...' loads every package swag will parse. If that cannot resolve, --parseDependency would emit a degraded schema and the diff would blame the committed file. * fix(openapi): gate the contract, not the bytes of the generated file The byte gate this replaces was red in CI and could not be made green. Its diff was `format: int64` missing from a dozen map[string]int64 fields — languages, by_scan_type, by_schedule_type — all describing identical Go types. Ruled out, with evidence: swag version drift (pinned v1.16.4 in both places, binary selection verified), Go toolchain drift (1.26.5 locally and in CI), and a cold module cache (generating with a fresh GOMODCACHE plus `go mod download` reproduces the committed spec exactly, so the cache was never the variable). swag is simply not hermetic across environments. A byte comparison of a 10,000-line generated artifact therefore fails on a difference that describes no disagreement about the API, that the developer who trips it cannot resolve, and whose only available fix is to weaken or delete the check. A gate people have to route around is worse than no gate. So gate the property the work was actually about. Every bug that motivated this was structural, not textual: 30 documented paths with no route in the repository, and 40 real endpoints missing including the whole /notifications API. tools/lint/openapicontract compares three sets: A. annotations == spec Every // @router is in the committed spec and every spec operation is an annotation. This is what makes the spec generated: a path cannot be hand-added, and an annotation cannot change without `make swagger`. B. spec ⊆ routes Every documented operation has a registered route. The phantom check. C. routes ⊆ spec ∪ baseline Every registered route is documented or listed in api/openapi/undocumented-routes.txt. 439 of 878 routes carry no annotation today; annotating them is a separate effort, so the baseline freezes the debt where a reviewer can see it. A NEW undocumented route fails, which is the case that actually hurt — that is how the entire /notifications API, GET /auth/providers and GET /scans/coverage stayed invisible to every client. The baseline cannot rot either: an entry that no longer names a real route is itself a failure, so a removed or newly-documented route must be pruned. Routes are read from the AST, not grepped. Registration nests through Group("/api/v1/x", func(r Router){ r.GET("/y", h) }), and a regex cannot tell which group a call belongs to. Set comparison is stable across swag's formatting, so the spec is now generated-but-not-byte-gated: cosmetic churn in swag's output will not fail this. What cannot drift is the set of operations, which is the part a generated client depends on. Still unconditional — no if:, no base-ref lookup — and it fails rather than skips when the spec or the baseline is missing. Proven to fail, each independently, with the failure naming the offending path: • hand-adding /me/hand-edited-phantom to the spec → A and B • pointing an @router at an unrouted path + make swagger → B • adding POST /me/event-types/brand-new-undocumented to a route group → C • a baseline entry that matches no route → C --------- Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
#407 landed with a comment claiming a cold module cache was why the generated spec differed between a developer machine and CI. That was the hypothesis while the gate was still a byte diff, and it was wrong — generating with a fresh GOMODCACHE plus go mod download reproduces the committed spec exactly. The download is still worth doing: --parseDependency has to read ctis source for the asset-type enum descriptions, and without the flag swag fails outright on json.RawMessage. But it is not a determinism fix, and the comment should not imply the byte output is reproducible when the gate deliberately does not assume that. Text only; the commit that carried this missed the squash of #407 by a few minutes. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…nts (#412) All three approval_* notifications are undeliverable on a default configuration, so a finding awaiting a decision blocks forever and nobody is told. Two independent gates run before a notification is delivered: ShouldNotifyEventType empty enabled_event_types -> the platform defaults ShouldNotify empty enabled_severities -> critical + high only DefaultEnabledEventTypes deliberately turns EventTypeApprovalRequested ON. The reasoning is recorded next to it: "An approval request is addressed to a human; if it reaches nobody the finding stays blocked indefinitely." The enqueue site then stamps a constant severity — "medium" for requested and rejected, "low" for approved (finding/vulnerability_service.go:2355, :2458, :2542). Neither is in the default critical+high set, so the severity gate drops every one of them. The event-type gate was opened on purpose and the severity gate closed it again, producing exactly the outcome that comment exists to prevent. The root cause is that EnqueueParams.Severity carries two different things. For new_finding, sla_breach and friends it IS the finding's severity, and an operator who leaves the filter at its default is saying "only critical and high findings" — honoring that is the whole point. For approval lifecycle events it is a constant chosen by the enqueue site that describes no finding at all, and no operator ever asked to suppress it. So the fix is not to bump the constants to "high", which would just launder a workflow event through a field that does not apply to it. SeverityFilterApplies decides per event type whether the severity gate is meaningful, and the approval trio is exempt. They remain fully controllable through the event-type filter, which is the switch that actually means "I do not want these". SendNotificationInput gains EventType so the single-integration path makes the same decision as the broadcast path. Empty is treated as filterable, so every existing caller keeps its current behavior. Adds a completeness gate: a new event type must be classified as severity-bearing or not, and the build fails if one is added without that decision. Verified to fire by removing a classification — it names the offending event type. Inheriting the default silently is how this bug happened. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
…s reconciler promise (#413) Migration 000155 says endpoint_asset_id is nullable because "during onboarding the agent may not yet know its asset UUID. A nightly reconciler job pairs events with assets by agent_id." No such job was ever written, and it cannot be. There is no join key: `agents` has no asset column, `assets` has no agent column, and there is no join table — so there is nothing to pair BY. Checked every promised background job in every migration comment (15 of them); this is the only one with no implementation. The other 14 all exist and are started. It is also the wrong idea. Only the producer knows which endpoint an event describes. An EDR/XDR forwarder reports on many hosts, so even the emitting agent's own hostname is not the answer. The server cannot infer this after the fact, at any point, by any means. So a NULL endpoint_asset_id is permanent, not a pending state — and the consequence is invisible. The event is stored, the response says accepted, and the IOC correlator still matches it because it keys on values inside the event. What silently does not happen is every asset-scoped read: Stage-4 detection correlation's heuristic fallback (CountNearTarget, by asset + window) and the per-asset Stage-6 dashboards. Half the feature does not apply, and nothing says so. Two changes, no new background job — building a reaper with no join key, for a stream that currently has no producer at all, would be adding to the silently-inert class rather than fixing it: 1. The migration comment retracts the promise and records why it cannot be kept, so the next person does not go looking for a job that was never possible. 2. The ingest response gains `unpaired`, counting ACCEPTED events that arrived without an asset link, plus a WARN naming the impact. A producer integrating today sees the degradation on the response it already reads instead of discovering months later that asset-scoped correlation never applied to its data. Rejected counts are excluded: those events were never stored, and counting them would send a producer looking for a configuration problem that is really a validation error. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
#414) The audit hash chain is keyed by tenant. Authentication events have no tenant — at login a user may belong to several tenants and has not chosen one yet — so appendChainEntry returned early for them: tenantPtr := log.TenantID() if tenantPtr == nil { return // system-level events bypass the per-tenant chain } On the live database that is 925 of 1075 audit rows, 86%: every auth.login (814), auth.register (87), auth.failed (20) and auth.logout (4). None of them carried any tamper evidence. An intruder with database access could delete the record of their own login, or the failed attempts that preceded it, and GET /audit-logs/verify would report the trail intact — because it only ever walked rows that were chained. Nothing documented this. It was a consequence of the per-tenant design, not a decision: migration 000154 describes the chain as per-tenant and says nothing about excluding authentication. Tenant-less events now extend a dedicated system chain. Why a sentinel tenant id rather than making audit_log_chain.tenant_id nullable: that column is a tenant-isolation boundary and loosening it is the more dangerous change. All-Fs is deliberate — its UUID version nibble is 'f', and uuid.NewV7 / uuid.New can only ever emit 7 or 4 there, so no generated id can collide with it. The all-ZEROS UUID was rejected for the opposite reason: it is the zero value of shared.ID, which several call sites already test with IsZero() to mean "unset". Three things had to change together, and any one of them alone would have been worse than the bug: 1. appendChainEntry appends tenant-less events to SystemChainTenantID. 2. The verifier walks that chain. ListActiveTenantIDs can never return it — it is not a tenant — so the controller adds it explicitly, and FIRST: a run cut short by its context deadline would otherwise skip whatever is last, and this is the chain an intruder has the most reason to edit. Writing hashes nobody checks is not tamper evidence. 3. VerifyChain resolves system entries with a new GetSystemByID (WHERE tenant_id IS NULL) instead of the tenant-scoped getter, which cannot see those rows and would have reported every single one as audit_log_missing — a fabricated tamper signal on the control that exists to detect real ones. GetSystemByID is a separate repository method rather than a relaxed GetByTenantAndID on purpose: that one is a tenant-isolation boundary, and widening it so a sentinel also matches NULL rows is exactly the kind of change that later leaks a real tenant's rows. GetSystemByID can only ever return rows with no tenant. Tests run through the real repository against a real database, because the question is not "does the Go branch take the right path" but "does a row land in audit_log_chain" — and the reason this gap survived is that each component was individually correct. Verified fail-before/pass-after by restoring the early return: "no chain row for a tenant-less auth event" and "the system chain verified 0 entries". Tradeoff worth naming: every login now takes chainMu and does one LatestChainHash read plus one insert, where before it did neither. All authentication auditing serialises on a single chain. Login rate bounds it and the existing per-tenant path already had the same shape, but on a high-login-rate deployment this is the thing to watch. Co-authored-by: Nguyen Manh <0xmanhnv@gmail.com>
The previous release was squash-merged, so main's HEAD has a single parent and git cannot see that develop already contains it. Without this commit, merging develop into main reports conflicts that are not real disagreements. -s ours keeps develop's tree byte-for-byte and only records main as a parent. Generated by scripts/release-branch.sh.
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.
Release of everything on
developsince v0.4.0. 57 files differ from the v0.4.0 tag (measured by content — the commit count between the tags is meaningless here, see below).What this ships
Five defects where the feature looked configured and was inert, plus the CTEM Stage-4 work. Each was verified against live data, not inferred from code.
Silent failures, user-visible
CommandExpirationCheckerruns on a 60s tick in every deployment and had never expired anything:FindExpiredrequiresexpires_at IS NOT NULLand no creation path ever set it — live: 21 commands, 0 with an expiry, 2 pending for 64 days. Sopipeline.OnStepFailed(..., "COMMAND_EXPIRED")had never fired, and a run whose command went unanswered hung until a generic timeout. Default now applied inNewCommand(the single seam), 48h, chosen to sit beyond every other timeout in the path.approval_*events dropped on a default configuration. The event-type gate turnsapproval_requestedon deliberately — "if it reaches nobody the finding stays blocked indefinitely" — and the severity gate closed it again, because the enqueue site stamps"medium"and an emptyenabled_severitiesmeans critical+high only. A finding awaiting a decision blocked forever with nobody told.auth.login,auth.register,auth.failed— were never chained. An intruder with database access could delete the record of their own login andGET /audit-logs/verifywould report the trail intact.verified: trueand emitted "target reachable" as a detection result from a TCP connect, driving the UIs "N% of simulated attacks were detected" toward zero — a confident report of total control failure produced by a probe that never looked at a control.CTEM Stage-4 (#410)
Answers "did our controls react?" — a
detection_statusvocabulary deliberately disjoint fromoutcome, becauseoutcomedescribes the target and detection describes our sensors, and the two have opposite polarity. The correlator refuses to saynot_observeduntil it has positively confirmed telemetry is flowing; with no EDR connected every verdict isno_telemetry_source(= unknown), never a false detection gap.Correctness of the tooling itself
unpairedso the degradation is visible.postgres://…/openctem, so an unconfiguredgo test ./...wrote rows and ran cross-tenant sweeps against the production database. Plus advisory-lock isolation for the cross-package sweep race./credentialswith a different endpoints schema.Migration
000203 (
validation_detection_status) — additive only: three columns with defaults, one CHECK, two indexes. No destructive operation;scripts/check-migrations.shpasses. Existingvalidation_evidencerows default tonot_evaluatedrather than being backfilled with a verdict nobody measured.Already applied to the live database (202 → 203, verified: columns, constraint, indexes present; 5 pre-existing rows all
not_evaluated), and the running API confirmsapplied_version=203 latest_migration=203.About this branch
Built with
make release-branch VERSION=v0.5.0. The tree isdevelops byte for byte; onemerge -s ourscommit recordsmainas a parent.This exists because releases here are squash-merged, which leaves
mainwith a single parent so git cannot see thatdevelopalready contains it — the next release then reports dozens of phantom conflicts. That has happened four times. Mergingmainback intodevelopdoes not stick, because the following release squashes again.Verified before merging that 0 files exist in
mainbut not indevelop, so-s oursdiscards nothing. Both properties are asserted by the script, and the resulting tree equality was checked directly rather than trusting ancestry.