Skip to content

feat: add plugin schema#37

Open
fzipi wants to merge 9 commits into
mainfrom
feat/add-plugin-schema
Open

feat: add plugin schema#37
fzipi wants to merge 9 commits into
mainfrom
feat/add-plugin-schema

Conversation

@fzipi
Copy link
Copy Markdown
Member

@fzipi fzipi commented Apr 13, 2026

what

  • add plugin schema for discovery and configuration

why

  • enhance plugin onboarding experience

Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a JSON Schema and accompanying documentation/examples to standardize CRS plugin discovery and configuration via a plugin.yaml descriptor.

Changes:

  • Introduce plugin-schema.json (JSON Schema draft 2020-12) for validating plugin descriptors.
  • Add documentation describing the schema fields and intended usage (docs/plugin-descriptor-schema.md).
  • Provide example plugin descriptors under examples/ for reference and onboarding.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
plugin-schema.json Defines the structure/constraints for plugin.yaml plugin descriptors.
docs/plugin-descriptor-schema.md Documents the schema fields, intent, and rollout plan.
examples/wordpress-rule-exclusions-plugin.yaml Example descriptor for a rule-exclusion plugin.
examples/fake-bot-plugin.yaml Example descriptor for a detection plugin with multiple variables.
examples/body-decompress-plugin.yaml Example descriptor for a utility plugin with numeric bounds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugin-schema.json
Comment thread plugin-schema.json
Comment thread plugin-schema.json
Comment thread plugin-schema.json Outdated
Comment thread examples/body-decompress-plugin.yaml Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
@fzipi
Copy link
Copy Markdown
Member Author

fzipi commented Apr 13, 2026

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown

Copilot AI commented Apr 13, 2026

@copilot apply changes based on the comments in this thread

Applied all changes from the review in commit bc82781:

  • configuration.variables: added minItems: 1 to prevent empty variable lists
  • default/example type validation: added if/then conditionals for all four types — boolean values must be integer 0 or 1; integer values must be integers; string and enum values must be strings
  • allowed_values: added items: {type: string} and a conditional requiring it when type is "enum", and forbidding it for non-enum types
  • min/max: changed from type: number to type: integer and added a conditional that forbids them on non-integer variable types
  • Example fix: renamed tx.body-decompress-plugin_enabletx.body-decompress-plugin_enabled to match the documented convention
  • Docs: updated the boolean type description to clarify values are integers (0/1) following the ModSecurity convention

@fzipi fzipi requested a review from theseion April 13, 2026 21:44
Version is derived from GitHub release tags at query time. Embedding it
in plugin.yaml would inevitably drift as developers forget to update it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@theseion theseion left a comment

Choose a reason for hiding this comment

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

The registry will also need to have a new way to register plugins, i.e., a YAML file that contains an entry for each GitHub repository. That means that at least the repository URL in the schema is an unnecessary duplicate.

We should make it clear that we currently only support GitHub repositories.

Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
fzipi and others added 2 commits April 19, 2026 20:46
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
- Remove `files` section from schema, docs, template, and examples.
  File paths follow the CRS naming convention and can be derived by
  tooling from the plugin name.
- Clarify `engines` field in docs as an enum (matching `category`
  wording), since the schema already restricts values.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md
@theseion
Copy link
Copy Markdown
Contributor

Do you agree with what I wrote here? Is I wrote there, we should also make it clear that we only support GitHub atm.

@fzipi
Copy link
Copy Markdown
Member Author

fzipi commented Apr 30, 2026

I agree on allowing only github repos for now.

Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@fzipi
Copy link
Copy Markdown
Member Author

fzipi commented Apr 30, 2026

@copilot fix all table formatting and add a similar section for the engines category taxonomy.

Copy link
Copy Markdown

Copilot AI commented Apr 30, 2026

@copilot fix all table formatting and add a similar section for the engines category taxonomy.

Done in commit eae1216:

  • Fixed table alignment for separator, prefix, and suffix rows in the variable definition table
  • Added an "Engine taxonomy" section under Design Decisions (parallel to the existing "Category taxonomy" section) listing all four supported values (modsecurity2, modsecurity3, coraza, all) with descriptions

Do you agree with what I wrote here? Is I wrote there, we should also make it clear that we only support GitHub a...

Agreed — the repository URL in plugin.yaml can be used as the lookup key until a separate registry YAML exists. For now, in commit eae1216 the repository field in the schema has been restricted to a GitHub owner/repo URL pattern (^https://github\.com/[^/]+/[^/]+/?$), and the docs field description has been updated to note that only GitHub repositories are currently supported.

Comment thread docs/plugin-descriptor-schema.md Outdated
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@fzipi fzipi requested a review from theseion May 25, 2026 11:26
| `category` | no | Functional category: `rule-exclusion`, `detection`, `protection`, `utility`, `logging`, or `performance`. |
| `status` | yes | Maturity level: `tested`, `being-tested`, `untested`, or `draft`. |
| `license` | yes | SPDX license identifier (e.g., `Apache-2.0`, `GPL-2.0-only`). |
| `authors` | yes | List of author objects with `name` (required), `email` and `url` (optional). |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
| `authors` | yes | List of author objects with `name` (required), `email` and `url` (optional). |
| `authors` | yes | List of author objects with `name` (required), `email` (required) and `url` (optional). |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmmm, this might be problematic if authors don't want to have a public way of contacting them, as spam might get into their inboxes, and/or other problems. Do we want to require it?

| `authors` | yes | List of author objects with `name` (required), `email` and `url` (optional). |
| `repository` | yes | URL of the plugin GitHub repository (only GitHub repositories are currently supported). |
| `homepage` | no | URL to documentation or project site. |
| `keywords` | no | Tags for discovery and categorization. |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In what format?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What do you mean? String? List?

Comment thread docs/plugin-descriptor-schema.md Outdated
| Field | Description |
|-------------|-------------|
| `file` | Path to the `-config.conf` file relative to the plugin root. |
| `variables` | List of transaction variable definitions (see below). |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In what format?

Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
Comment thread docs/plugin-descriptor-schema.md Outdated
- Add `list` to variable type enum in schema and docs
- Add `separator`, `prefix`, `suffix` properties to variable schema
- Require `separator` when type is `list`; forbid separator/prefix/suffix for non-list types
- Add `all` to engines enum (schema now matches docs)
- Update docs: required column for `allowed_values` (enum) and `separator` (list)
- Clarify `prefix`/`suffix` descriptions (prepended/appended per item)
- Clarify `variables` field description with YAML sequence note
- Fix "four types" → "five types" to include `list`

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@fzipi
Copy link
Copy Markdown
Member Author

fzipi commented Jun 3, 2026

Addressed the open review comments in the latest commit:

Schema changes (plugin-schema.json):

  • Added list to the variable type enum
  • Added separator, prefix, suffix properties to the variable schema
  • separator is now required when type is list; separator/prefix/suffix are forbidden for non-list types
  • Added all to the engines enum so the schema matches the documented behavior

Docs changes (docs/plugin-descriptor-schema.md):

  • Updated allowed_values required column to yes (for "enum") — matches schema conditional
  • Updated separator required column to yes (for "list") — matches schema conditional
  • Clarified prefix/suffix descriptions: "prepended/appended to each list item"
  • Clarified variables field description: "YAML sequence of transaction variable definitions"
  • Fixed "four types" → "five types" to account for list

Still open (active discussion, no consensus yet):

  • Whether email should be required for authors
  • Format/meaning of keywords

Comment thread docs/plugin-descriptor-schema.md Outdated

#### Variable Types

The four types cover all patterns found across existing CRS plugins:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm using another type of variables in few of 'my' plugins - it's some kind of an array, used in Lua plugins. For example:

tx.false-positive-report-plugin_smtp_cc_1=first@example.com
tx.false-positive-report-plugin_smtp_cc_2=second@example.com
tx.false-positive-report-plugin_smtp_cc_3=third@example.com

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you point me to the code to take a look?

Comment thread docs/plugin-descriptor-schema.md Outdated
| `allowed_values` | no | List of valid values when type is `enum`. |
| `separator` | no | String used to separate multiple entries when type is `list`. |
| `prefix` | no | String marking the beginning of a list entry when type is `list`. |
| `suffix` | no | String marking the end of a list entry when type is `list`. |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
| `suffix` | no | String marking the end of a list entry when type is `list`. |
| `suffix` | no | String marking the end of a list entry when type is `list` (see below). |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants