Repository navigation
Conversation
vacuum reported 313 warnings (score 0/100). Most were gaps in what the spec declares: - 400 on the 32 operations whose parameters or body the gateway validates, and 500 on /ws and the public emoji routes. - X-RateLimit-* headers on the 47 rate-limited responses that lacked them. Barbacane sends them on allowed responses from the rate-limit fix in barbacane-dev/barbacane#241 on. - Patterns for IDs, cursors, limits and webhook tokens, matching the formats the server emits and parses; format: email on AdminUser.email. - Bounds on the rate-limit headers and the int64 counters, and the policy header's real format (`<name>;q=<quota>;w=<window>`). - Descriptions on 15 request bodies and 4 schemas, `security: []` and a 502 on the SPA routes, and the unused PinnedMessage schema removed. specs/.vacuum-ignore.yaml lists the findings that do not apply, each with its reason: free text, the SPA routes that run no middleware, operations without input, the 101 WebSocket upgrade. make lint-spec and CI pass it. Left visible: the webhook trigger has no rate limit (its `[]` removes rate-limit along with oidc-auth), and 116 missing examples. The 25 barbacane-auth-opt-out-explicit findings are a ruleset bug, removed in barbacane-dev/barbacane#242. Score 59/100, 118 warnings; the gateway artifact compiles. Signed-off-by: Nicolas Dreno <nicolas.dreno@barbacane.dev>
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: barbacane-dev/burst/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
vacuum scored the spec 0/100 with 313 warnings. The CI gate counts only errors, so it passed. Most warnings were real gaps in what the spec declares. This PR fixes those and lists the rest, each with its reason. Score now 59/100, 118 warnings.
Changes
Declared what the API actually does
400(MalformedRequest) on the 32 operations whose parameters or body the gateway validates.PATCH /api/users/mehad a body and no 400.500on/wsand the public emoji routes.502on the SPA route, which is what the s3 dispatcher returns when the bucket is unreachable.X-RateLimit-*headers on the 47 rate-limited responses that lacked them.security: []on the three SPA routes, which are public by design.Constraints that match the server (
crates/burst-core/src/id.rs, theparse_*_idhelpers)usr_/ch_/exp_-prefixed on responses, a bare UUID for emoji and audit-entry IDs.limit:^[1-9][0-9]{0,2}$on the five endpoints where it had no pattern, matching the other endpoints.^[0-9a-f]{64}$. AuditactionandtargetType: patterns.AdminUser.email:format: email.<name>;q=<quota>;w=<window>. The previous example100;w=60was not what the plugin sends.Cleanup: descriptions on 15 request bodies and 4 schemas, descriptions for the catch-all
pathparameters, and the unusedPinnedMessageschema removed.specs/.vacuum-ignore.yaml: findings that don't apply, grouped with reasons: free text, cursors the server never sets, operations without input, SPA routes that run no middleware, and the101WebSocket upgrade.make lint-spec, CI andCLAUDE.mdpass--ignore-file.Behavior changes at the gateway
Request validation now rejects a few inputs it accepted before, with a 400 in each case:
limitof0, or with a leading zero, on those five endpoints;emojiIdoruserIdon the admin delete/update routes;threadIdmultipart field that isn't a message ID.The server already answered all of these with 400, except that its UUID parser also accepts uppercase hex, and the spec's patterns (like the existing shared ID parameters) require lowercase.
Depends on
X-RateLimit-*on allowed responses. Until Burst runs a Barbacane release containing it, those declarations (old and new) describe headers that don't arrive. BumpingBARBACANE_VERSIONthen closes that.barbacane-auth-opt-out-explicit, a false positive under middleware merging (the 25 remaining infos). CI fetches the ruleset from docs.barbacane.dev, so they disappear once that deploys.Left visible on purpose
POST /api/webhooks/{webhookId}/triggerhas no rate limiting. Itsx-barbacane-middlewares: []removesrate-limitalong withoidc-auth, and the server doesn't limit it either. This is a public, token-authenticated endpoint that creates messages. Needs a decision (see below).good first issue.cursor/limitwording across endpoints.Testing
make lint-specpasses.make gateway-compilecompiles both artifacts (67 and 3 routes). The only warning is the existing E1033 on the webhook trigger.