feat(licensing): enforcement coverage + GET /capabilities (v0.7 criteria 2 and 3) - #770
Merged
Merged
Conversation
… 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.
…(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
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.
Closes the last two v0.7 exit criteria. Both turned out to be buildable without the route mapping I said they were blocked on: seven of the nine paid capabilities have no routes yet, so there was almost nothing to map.
Criterion 2: fail the build when a paid route ships ungated
Checks both directions:
The second direction found a gap I created three PRs ago: the attestation gate from #766 is enforced in the handler and declared nowhere. Fixed here.
It passes almost trivially today. That is the point. It exists so that when bulk remediation lands in v0.9 and someone forgets the gate, the build fails instead of the capability shipping free.
A new annotation, and the distinction is load-bearing
x-required-featuremeans the whole route is paid.x-conditional-featuremeans paid for some requests, decided from the body.Reports are the live case: only the
attestationkind is entitled, so a route-level declaration would state in the public contract that a free route is paid. Collapsing the two 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, not review, and both the same class:
enforces()hardcoded theauth.prefix. Reusing it for license constants silently reported every license gate as missing.Negative-tested all three failure modes: unregistered flag, gate swapped to a different feature, gate enforced but undeclared. All three now fail; the middle one is the case that slipped through first time.
Criterion 3: GET /capabilities
Before this, a Community user could not tell a paid tier existed. The only way to discover an entitlement was to click something and collect a 402.
Built entirely from the registry plus the runtime check, so it cannot drift from what is actually gated.
Unauthenticated by design, like
GET /license. AC-03 asserts the no-disclosure property against the raw JSON, not a typed struct — a typed decode would silently ignore an added field, which is exactly the leak being guarded against.No quotas in the response, because there are no quotas in the product. A "78% of your host allowance" indicator would invent a limit that does not exist.
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. Marked out of scope in the spec so it does not read as finished.
Verification
New spec
api-capabilities1.0.0 registered inspecter.yaml;system-license-features1.1.0 -> 1.2.0. 117 specs, 117 passing. Fullgo test ./internal/...clean locally before pushing.