feat(licensing): gate the auditor attestation artifact behind compliance_attestation - #766
Merged
Merged
Conversation
…nce_attestation
The founder named "a signed OSCAL report for a compliance officer" as a moat
item, meaning something OpenWatch monetizes. Every report route was ungated:
all twelve, including /reports/{id}/export and the fleet OSCAL SAR face.
That collided with the opencore roadmap's LOCKED tiering rule, which names
reports explicitly among things that "stay free" because they already shipped
free. Honoring the lock literally would have made the clearest monetization
case permanently unmonetizable; ignoring it would revoke a shipped capability.
Resolved by splitting rather than revoking, and the axis matters. Scope was the
obvious choice and is wrong: an absent scope body means all hosts and all
frameworks, so a scope-based gate would have gated the default executive
summary, which is the everyday free artifact. The line is the report KIND.
free executive kind at ANY scope; per-scan and per-host OSCAL
(/scans/{id}/oscal, /scans/{id}/rules/{ruleId}/oscal); the
signing key; report list and read
enterprise the attestation kind
Kind is also exact rather than approximate: report.FaceOSCALSAR is
attestation-only (exportFleetOSCALSAR), so gating this one kind gates the fleet
OSCAL SAR and nothing else. A Community user keeps a free path to OSCAL
evidence per scan; what is paid is the fleet-wide signed package filed with an
auditor.
Gated at four sites, because any one alone is theatre:
- generation, on the requested kind
- export, on the STORED report's kind, or every attestation already in the
database stays freely exportable
- schedule creation, or the artifact is minted on a timer
- the dispatcher at FIRE time, or a schedule created while licensed keeps
producing the paid artifact after the licence lapses
Each of the four was negative-tested independently by neutering it while
keeping the build valid, so the assertion fires rather than the compiler.
The locked tiering rule is amended in opencore/ROADMAP.md rather than quietly
bent. A locked rule that gets silently excepted stops being a rule.
Registry-first: compliance_attestation added to licensing/features.yaml and
codegen'd before any route referenced it. Enterprise is now 9 flags; the free
set is unchanged, so system-license-features AC-13 still pins it.
Spec: api-reports 1.14.0 -> 1.15.0, C-19 and AC-25. Two id collisions caught by
the duplicate check before landing: C-09 already existed, and the renumbering
sed initially repointed AC-11's reference away from it. specter's
orphan_constraint gate caught the second.
116 specs, 116 passing. gofmt, go vet, go build, full go test ./internal/...
… the fire-time gate CI caught what the local suite could not: two DB-gated report-schedule suites use the `attestation` kind as their fixture, because it has the richest content. Gating that kind made both fail. They test schedule mechanics, not licensing, so they now run as a licensed deployment would. That needed an exported hook. license.EnableFeatureForTesting mirrors the existing SetVerificationKeyForTesting pattern and carries an explicit warning not to use it to paper over a gate: a test asserting an unlicensed caller is refused must not call it, because that is the case the gate exists for. Adds the behavioural half of AC-25 for the fire-time gate. The source inspection proves the check is written; this proves it works, which matters most here because the dispatcher gate is the one an operator cannot see. A schedule created while licensed would otherwise keep minting the paid artifact on a timer forever after the licence lapsed, with no request to refuse. The test asserts three things: unlicensed attestation never reaches Generate and skipping is not an error, the free `executive` kind is unaffected, and a LICENSED attestation does reach Generate, so the gate is proven to be the licence rather than the kind alone.
…fore freshAPIServer calls license.Init(), which resets state, so enabling the feature beforehand was wiped before the request ran and the suite still saw a 402. Enable after fixture setup in both suites; the dispatcher one happened to work only because freshPool does not init license, which is the kind of ordering dependency that fails later for an unrelated reason.
remyluslosius
force-pushed
the
feat/gate-attestation-reports
branch
from
July 29, 2026 23:36
bc52bfe to
f573da6
Compare
remyluslosius
added a commit
that referenced
this pull request
Jul 30, 2026
…#767) * ci: local test database so DB-gated suites run before push, not in CI The DB-gated suites do not run without OPENWATCH_TEST_DSN, so `make ci-local` silently skipped them and a change could look clean locally and fail in CI. That happened three times in one day on the v0.7 branches: the report-schedule suites (#766), the RBAC role-assign and token tests (#765), and the go mod tidy drift (#762). Every one was a real defect that a local run would have caught. scripts/test-db.sh mirrors the CI service container exactly. Image and credentials match .github/workflows/go-ci.yml, because if they drift then "passes locally" stops meaning "passes in CI", which is the whole point. Two settings are load-bearing and are commented as such in the script: max_connections=400. The default 100 is not enough. The suite runs packages in parallel and internal/db/dbtest clones a template database per package, so a default container deadlocks: the first DB test times out at 120s and the rest cascade. Found this the honest way, by hitting it. It looks exactly like a real failure and is not one, which is worse than no local database at all. fsync, synchronous_commit and full_page_writes off. The database is disposable; durability buys nothing and costs most of the runtime. Port defaults to 5455 rather than 5432: on this workstation 5432 is the dev database and 5433 is hanalyx-postgres. Override with TEST_DB_PORT. The database name ends in _test so the internal/db guard accepts it without the ALLOW_NONTEST bypass, which we never want set. `make ci-local` now warns when OPENWATCH_TEST_DSN is unset and says exactly how to fix it. A silent skip is what let this through repeatedly; a loud skip is the minimum. Verified: full `go test ./internal/...` against the container is clean, and the three suites CI previously caught now run and pass locally. * ci: un-ignore scripts/test-db.sh so the Makefile target has a script The blanket `*test*.sh` in .gitignore swallowed scripts/test-db.sh. `git add` refused it, the ci-local change shipped referencing a script that was never committed, and nothing failed loudly: the PR just contained a Makefile target pointing at a file no one else would have. Same trap as the blanket `*.spec` rule that drops new packaging spec files. Negate deliberately and verify with `git check-ignore -v <path>` after adding anything to scripts/ whose name contains "test".
remyluslosius
added a commit
that referenced
this pull request
Jul 30, 2026
…ria 2 and 3) (#770) * test(licensing): fail the build when a paid route ships ungated (v0.7 criterion 2) The licensing machinery is complete and wired to exactly one demo route. Seven of the nine paid capabilities have no code yet, so this check passes almost trivially today. That is the point: it is not here to find current gaps, it is here so that when bulk remediation lands in v0.9 and someone forgets the gate, the build fails instead of the capability shipping free. Checks both directions: declared -> enforced a route naming a feature must name a REGISTERED one and must enforce it in that route's own call graph enforced -> declared a paid feature enforced in the server package must be declared by some route, or the contract hides a paywall The second direction found a gap I created three PRs ago: the attestation gate from #766 is enforced in the handler and declared nowhere, so the public contract was silent about a paywall that exists. Fixed here. Adds x-conditional-feature alongside x-required-feature, and the distinction is load-bearing. The attestation gate depends on the request BODY (only report kind `attestation` is entitled), so a route-level x-required-feature would declare a free route paid. Collapsing the two annotations forces a choice between lying about a free route and leaving a real gate undeclared, and the second is exactly what happened. Two mistakes in my own first cut, both caught by negative-testing rather than by review, both the same class: The conditional branch searched the whole PACKAGE for the constant. That passed a route whose gate had been swapped to a different feature, because the right constant still appeared elsewhere. A loose match in a security check is worse than none: it reads as coverage. enforces() hardcoded the "auth." prefix. Reusing it for license constants silently reported every license gate as missing. Now parameterised, and it follows package-level helpers as well as methods, since RBAC delegates to a method and the license gate to a package function. Negative-tested all three failure modes: an unregistered flag, a gate swapped to a different feature, and a gate enforced but undeclared. All three fail the AC; the middle one is the case that slipped through the first implementation. Spec: system-license-features 1.1.0 -> 1.2.0, AC-14. 116 specs, 116 passing. * feat(licensing): GET /capabilities so a customer can see the paywall (v0.7 criterion 3) Before this, a Community user could not tell that a paid tier existed. The licensing machinery shipped complete and earned nothing, partly because there was no way for the interface to lock or upsell a control: the only way to discover an entitlement was to click something and collect a 402. Built entirely from licensing/features.yaml plus the runtime entitlement check, so a capability cannot be reported without being registered first. Same registry-first rule the gating side follows, and it means this endpoint cannot drift from what is actually gated. Unauthenticated by design, like GET /license: a pre-login screen needs to know what the deployment offers. It discloses capability ids, their tier, and availability here, and nothing about who holds the license. AC-03 asserts that against the RAW JSON rather than a typed struct, because a typed decode silently ignores an added field, which is exactly the leak being guarded against. No quotas in the response, because there are no quotas in the product. Tiering is by capability alone: OpenWatch does not cap hosts, scans or users, so a percent-of-allowance indicator would invent a limit that does not exist. Order is stable by id. Go map iteration would make the response non-deterministic and churn any client that diffs it. New spec api-capabilities 1.0.0, registered in specter.yaml. 117 specs. NOT DONE: the lock and upsell UI. The endpoint is the contract the UI needs, but where a lock appears, what the upsell says, and how a grace-period banner reads are product decisions rather than plumbing. Explicitly out of scope in the spec so it does not read as finished.
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.
Why
You named "a signed OSCAL report for a compliance officer" as a moat item, meaning something OpenWatch monetizes. Every report route was ungated: all twelve, including
/reports/{id}/exportand the fleet OSCAL SAR face.That collided head-on with the opencore roadmap's locked tiering rule, which names reports explicitly among things that "stay free" because they already shipped free. Honoring the lock literally would make your clearest monetization case permanently unmonetizable. Ignoring it would revoke a shipped capability.
The split, and why the axis matters
Resolved by splitting rather than revoking. Scope was the obvious axis and it is wrong: an absent scope body means all hosts and all frameworks, so a scope-based gate would have gated the default executive summary, which is the everyday free artifact.
The line is the report kind:
executivekind at any scope; per-scan and per-host OSCAL (/scans/{id}/oscal,/scans/{id}/rules/{ruleId}/oscal); the signing key; report list and readattestationkindKind is also exact rather than approximate:
report.FaceOSCALSARis attestation-only (exportFleetOSCALSAR), so gating this one kind gates the fleet OSCAL SAR and nothing else.A Community user keeps a free path to OSCAL evidence per scan. What is paid is the fleet-wide signed package filed with an auditor.
Four gate sites, because any one alone is theatre
Each was negative-tested independently, neutering it while keeping the build valid so the assertion fires rather than the compiler.
The locked rule is amended, not bent
Recorded as an explicit exception in
opencore/ROADMAP.md(local-only, gitignored). A locked rule that gets silently excepted stops being a rule.Registry-first
compliance_attestationadded tolicensing/features.yamland codegen'd before any route referenced it. Enterprise is now 9 flags; the free set is unchanged, sosystem-license-featuresAC-13 still pins it.SDD
api-reports1.14.0 -> 1.15.0, C-19 and AC-25. 116 specs, 116 passing.Two id collisions caught before landing:
C-09already existed (caught by the duplicate check I added after the AC-13 collision in #763), and the renumbering initially repointed AC-11's reference away from it.specter'sorphan_constraintgate caught the second one.Verification
gofmt,go vet,go build ./..., fullgo test ./internal/..., spec gate.