Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions template/.config/vacuum-functions/bodyDescription.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Vacuum custom JS function (goja runtime: no npm, no require/import — plain self-contained JS).
//
// Replaces the built-in `operation-description` rule's requestBody/response checks. That rule only looks
// at requestBody.description / response.description directly and does not resolve $ref, so it can't see
// a description that comes from the referenced schema's docstring. That forced every route to carry a
// hand-typed description duplicating the payload/response model's own docstring.
//
// Vacuum resolves $ref before handing the node to a custom function, so `input.content[mediaType].schema`
// here is already the dereferenced schema — its `description` (from the pydantic model's docstring) is
// visible. This function passes if there's a description directly on the body object OR on any of its
// content schemas, and only flags a body with no description anywhere.
//
// An Optional payload (`payload: Model | None = None`) produces an `anyOf: [<model schema>, {type: null}]`
// wrapper instead of the model schema directly, so the description lives one level deeper — branches of
// anyOf/oneOf are checked too.

function getSchema() {
return { name: "bodyDescription" };
}

function hasOwnDescription(node) {
return !!node && typeof node.description === "string" && node.description.length > 0;
}

// An Optional[Model] field (e.g. `payload: Model | None = None`) resolves to
// {"anyOf": [{...model schema, with its docstring as description...}, {"type": "null"}]} — the
// description lives on a branch, not on the anyOf node itself, so branches must be checked too.
function schemaHasDescription(schema) {
if (!schema || typeof schema !== "object") {
return false;
}
if (hasOwnDescription(schema)) {
return true;
}
var branches = schema.anyOf || schema.oneOf;
if (Array.isArray(branches)) {
for (var i = 0; i < branches.length; i++) {
if (schemaHasDescription(branches[i])) {
return true;
}
}
}
return false;
}
Comment thread
zendern marked this conversation as resolved.

function runRule(input) {
if (!input || typeof input !== "object") {
return [];
}
if (hasOwnDescription(input)) {
return [];
}
var content = input.content;
if (content && typeof content === "object") {
for (var mediaType in content) {
var schema = content[mediaType] && content[mediaType].schema;
if (schemaHasDescription(schema)) {
return [];
}
}
}
return [{ message: "body is missing a description (own, or via its schema's docstring)" }];
}
92 changes: 92 additions & 0 deletions template/.config/vacuum-functions/bodyExample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Vacuum custom JS function (goja runtime: no npm, no require/import — plain self-contained JS).
//
// Replaces the built-in `oas3-missing-example` rule's requestBody/response checks. That rule only looks
// for an `example`/`examples` key on the media type object itself (or, at the schema-property level, on
// that property's own node) and does not drill into a `$ref`'d schema's own fields. Every field-level
// `Field(examples=[...])` on a payload/response model is therefore invisible to it, which forced routes
// to carry a hand-typed body example duplicating examples that already exist on the model.
//
// Vacuum resolves $ref before handing the node to a custom function, so `input.content[mediaType].schema`
// here is already the dereferenced schema. This function passes if there's an example directly on the
// body/media object, or anywhere within its (dereferenced) schema: on the schema itself, on any of its
// properties (recursively), on array items, or on an anyOf/oneOf branch (the shape of an `Optional[Model]`
// field). It only flags a body with no example reachable anywhere in that tree.

function getSchema() {
return { name: "bodyExample" };
}

function hasOwnExample(node) {
if (!node || typeof node !== "object") {
return false;
}
if (Array.isArray(node.examples) && node.examples.length > 0) {
return true;
}
return "example" in node && node.example !== undefined;
}
Comment thread
zendern marked this conversation as resolved.

// Walks a resolved schema looking for an example anywhere within it: on itself, on a property
// (recursively, since a nested model's own field examples count), inside array items, or on an
// anyOf/oneOf branch (how an `Optional[Model]` field is represented). `seen` guards against infinite
// recursion on a self-referencing schema.
function schemaHasExample(schema, seen) {
if (!schema || typeof schema !== "object") {
return false;
}
if (seen.indexOf(schema) !== -1) {
return false;
}
seen.push(schema);

if (hasOwnExample(schema)) {
return true;
}

var branches = schema.anyOf || schema.oneOf || schema.allOf;
if (Array.isArray(branches)) {
for (var i = 0; i < branches.length; i++) {
if (schemaHasExample(branches[i], seen)) {
return true;
}
}
}

if (schema.items && schemaHasExample(schema.items, seen)) {
return true;
}

var properties = schema.properties;
if (properties && typeof properties === "object") {
for (var key in properties) {
if (schemaHasExample(properties[key], seen)) {
return true;
}
}
}

return false;
}

function runRule(input) {
if (!input || typeof input !== "object") {
return [];
}
var content = input.content;
if (!content || typeof content !== "object") {
return [];
}

var results = [];
for (var mediaType in content) {
var mediaObject = content[mediaType];
if (hasOwnExample(mediaObject)) {
continue;
}
if (schemaHasExample(mediaObject && mediaObject.schema, [])) {
continue;
}
results.push({ message: "body (" + mediaType + ") is missing an example, own or via its schema's fields" });
}
return results;
}
17 changes: 7 additions & 10 deletions template/.config/vacuum-ignore.yaml.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,14 @@
owasp-define-error-validation:
- "$.paths['/api/shutdown'].get.responses"

# ValidationError is generated by FastAPI (the 422 body); its `loc` union contains an int we cannot
# annotate with a format/bounds without owning the model, so exempt that one generated node.
# HTTPValidationError is generated by FastAPI; its `detail` property is exempt because we cannot attach a
# field-level example to a generated model (a schema-level example is injected in custom_openapi instead).{% endraw %}{% if backend_uses_graphql %}{% raw %}
# /api/graphql's 200 response body is arbitrary JSON shaped by the caller's GraphQL query/GraphiQL page;
# there is no single representative example to attach without misleadingly implying a fixed response shape.{% endraw %}{% endif %}{% raw %}
oas3-missing-example:
- "$.components.schemas['HTTPValidationError'].properties['detail']"{% endraw %}{% if backend_uses_graphql %}{% raw %}
- "$.paths['/api/graphql'].get.responses['200'].content['application/json']"
- "$.paths['/api/graphql'].post.responses['200'].content['application/json']"{% endraw %}{% endif %}{% raw %}
{% endraw %}{% if backend_uses_graphql %}{% raw %}# /api/graphql's 200 response body is arbitrary JSON shaped by the caller's GraphQL query/GraphiQL page;
# there is no single representative example to attach without misleadingly implying a fixed response shape.
response-body-example:
- "$.paths['/api/graphql'].get.responses['200']"
- "$.paths['/api/graphql'].post.responses['200']"

{% endraw %}{% endif %}{% raw %}# ValidationError is generated by FastAPI (the 422 body); its `loc` union contains an int we cannot
# annotate with a format/bounds without owning the model, so exempt that one generated node.
owasp-integer-format:
- "$.components.schemas['ValidationError'].properties['loc'].items.anyOf[1]"
owasp-integer-limit:
Expand Down
69 changes: 61 additions & 8 deletions template/.config/vacuum-ruleset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ rules:
# `Any`-typed fields, which are rare and usually unavoidable (e.g. the generated ValidationError.input).
oas-missing-type: "off"

# DISABLED: same false-positive family as `oas-missing-type` above — it checks requestBody.description
# / response.description directly and does not follow `$ref`, so it can't see that the payload/response
# model's own docstring already documents the body. That forced every route to hand-type a description
# duplicating the model docstring almost verbatim. Replaced below by `request-body-description` /
# `response-body-description`, which resolve the schema first and pass if either the body itself or its
# (dereferenced) schema has a description.
operation-description: "off"

# DISABLED: same false-positive family as `operation-description` above — it only checks for an
# `example`/`examples` key on the media type object (or, at the property level, on that property's own
# node) and does not resolve `$ref`, so it can't see a body/response model's own field-level examples.
# Replaced below by `request-body-example` / `response-body-example`, which resolve the schema first and
# pass if an example exists anywhere within it.
oas3-missing-example: "off"

# ENABLED: guarantees every tag used by an operation is declared at the top level with real metadata —
# this drives navigation/grouping in docs and Kiota-generated client namespaces.
# `true` pulls vacuum's built-in definition verbatim (it's an opt-in rule, absent from `recommended`).
Expand Down Expand Up @@ -81,7 +96,7 @@ rules:
path-param-snake-case:
description: Path parameters must be snake_case.
given: $.paths[*][*].parameters[?(@.in=='path')].name
severity: warn
severity: error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: do we have documentation somewhere of why we're changing these all to error (e.g. a link to the github issue you created)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue here daveshanley/vacuum#947 but TLDR; output is a little weird if they are not errors. So at least the ones that we control here we are bumping them up to error even though our fail-severity is warning so that if any of those fail they render as such.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know right now why we're doing it. I meant "should we add a note or link to the issue in the the code for our future selves?". Just nit though

then:
function: casing
functionOptions:
Expand All @@ -92,7 +107,7 @@ rules:
query-param-camel-case:
description: Query parameters must be camelCase.
given: $.paths[*][*].parameters[?(@.in=='query')].name
severity: warn
severity: error
then:
function: casing
functionOptions:
Expand All @@ -104,7 +119,7 @@ rules:
response-media-type-json:
description: Response bodies must be application/json (or application/problem+json for errors).
given: $.paths[*][*].responses[*].content[*]~
severity: warn
severity: error
then:
function: pattern
functionOptions:
Expand All @@ -115,7 +130,7 @@ rules:
request-media-type-json:
description: Request bodies must be application/json.
given: $.paths[*][*].requestBody.content[*]~
severity: warn
severity: error
then:
function: pattern
functionOptions:
Expand All @@ -130,7 +145,7 @@ rules:
description: POST request bodies must not require an id (backend-generated).
given: $.paths[*].post.requestBody.content.*.schema.required[*]
resolved: true
severity: warn
severity: error
then:
function: pattern
functionOptions:
Expand All @@ -142,7 +157,7 @@ rules:
no-action-verbs-in-path:
description: Path segments must be nouns, not action verbs.
given: $.paths[*]~
severity: warn
severity: error
then:
function: pattern
functionOptions:
Expand All @@ -157,7 +172,7 @@ rules:
location-header-on-201:
description: 201 Created responses must declare a Location header.
given: $.paths[*].post.responses['201']
severity: warn
severity: error
then:
field: headers.Location
function: truthy
Expand All @@ -171,6 +186,44 @@ rules:
resources-plural:
description: Path resource segments should be plural nouns.
given: $.paths[*]~
severity: warn
severity: error
then:
function: resourcesPlural

# ENABLED: replaces the requestBody half of the built-in `operation-description` (disabled above).
# Uses .config/vacuum-functions/bodyDescription.js, which resolves the requestBody's schema and passes
# if either the requestBody itself or its (dereferenced) schema has a description — so a payload
# model's docstring is enough, no hand-typed duplicate needed.
request-body-description:
description: Request bodies must have a description, either their own or via their schema's docstring.
given: $.paths[*][*].requestBody
severity: error
then:
function: bodyDescription

# ENABLED: same replacement as `request-body-description` above, for response bodies.
response-body-description:
description: Response bodies must have a description, either their own or via their schema's docstring.
given: $.paths[*][*].responses[*]
severity: error
then:
function: bodyDescription

# ENABLED: replaces the requestBody half of the built-in `oas3-missing-example` (disabled above). Uses
# .config/vacuum-functions/bodyExample.js, which resolves the requestBody's schema and passes if an
# example exists on the body itself or anywhere within its (dereferenced) schema — so field-level
# `Field(examples=[...])` on the payload model is enough, no hand-typed duplicate needed.
request-body-example:
description: Request bodies must have an example, either their own or via their schema's fields.
given: $.paths[*][*].requestBody
severity: error
then:
function: bodyExample

# ENABLED: same replacement as `request-body-example` above, for response bodies.
response-body-example:
description: Response bodies must have an example, either their own or via their schema's fields.
given: $.paths[*][*].responses[*]
severity: error
then:
function: bodyExample
3 changes: 3 additions & 0 deletions template/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ node_modules/
# Nuxt generate creates a symlink to the dist folder, which can't have trailing slash
dist

# vacuum OpenAPI lint reports (generated ad-hoc; the linter runs against the committed snapshot)
vacuum-report-*.json
Comment thread
ejfine marked this conversation as resolved.


# Infrastructure as Code
.terraform/
Expand Down
42 changes: 21 additions & 21 deletions template/.pre-commit-config.yaml.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,27 @@ repos:
# rev: b933184438555436e38621f46ceb0c417cbed400 # frozen: v1.13.0
# hooks:
# - id: zizmor
{% endraw %}{% if has_backend %}{% raw %}
- repo: local
hooks:
- id: vacuum-openapi
name: vacuum OpenAPI schema lint
# Lints the generated OpenAPI snapshot against the curated ruleset.
# `language: node` with additional_dependencies lets pre-commit install vacuum in an
# isolated env, so this runs in CI without any global/devcontainer install. Fails on warn-or-worse.
entry: vacuum lint --no-banner --no-style --fail-severity warn --min-score 100 -d --no-message --no-clip -f .config/vacuum-functions -r .config/vacuum-ruleset.yaml --ignore-file .config/vacuum-ignore.yaml backend/tests/unit/__snapshots__/test_basic_server_functionality/test_openapi_schema.json
language: node
additional_dependencies: ['@quobix/vacuum@{% endraw %}{{ vacuum_openapi_version }}{% raw %}']
files: |
(?x)^(
backend/tests/unit/__snapshots__/test_basic_server_functionality/test_openapi_schema\.json|
.config/vacuum-ruleset\.yaml|
.config/vacuum-ignore\.yaml|
.config/vacuum-functions/.*\.js
)$
pass_filenames: false
require_serial: true
verbose: true{% endraw %}{% endif %}{% raw %}

# Linting

Expand Down Expand Up @@ -339,27 +360,6 @@ repos:
# use require_serial so that script is only called once per commit
require_serial: true
# print the number of files as a sanity-check
verbose: true

- repo: local
hooks:
- id: vacuum-openapi
name: vacuum OpenAPI schema lint
# Lints the generated OpenAPI snapshot against the curated ruleset.
# `language: node` with additional_dependencies lets pre-commit install vacuum in an
# isolated env, so this runs in CI without any global/devcontainer install. Fails on warn-or-worse.
entry: vacuum lint --no-banner --no-style --fail-severity warn --min-score 100 -d --no-message --no-clip -f .config/vacuum-functions -r .config/vacuum-ruleset.yaml --ignore-file .config/vacuum-ignore.yaml backend/tests/unit/__snapshots__/test_basic_server_functionality/test_openapi_schema.json
language: node
additional_dependencies: ['@quobix/vacuum@{% endraw %}{{ vacuum_openapi_version }}{% raw %}']
files: |
(?x)^(
backend/tests/unit/__snapshots__/test_basic_server_functionality/test_openapi_schema\.json|
.config/vacuum-ruleset\.yaml|
.config/vacuum-ignore\.yaml|
.config/vacuum-functions/.*\.js
)$
pass_filenames: false
require_serial: true
verbose: true{% endraw %}{% endif %}{% raw %}

# Updating repo config/tooling files
Expand Down
Loading