docs: add ADRs 0013-0016, TODO.md, and consistency cleanups#71
Merged
Conversation
Adds four new ADRs covering structural concepts that were missing or under-specified in the prior set: - ADR-0013: Documentation Model — four tiers (ruleset/group/rule/directive) with doc-comments for narrative + structured fields for machine-readable content. Groups nest for conceptual cohesion; cross-cutting concepts (paranoia, severity) get their own documentation blocks. - ADR-0014: Imports and Modularity — explicit `import` directive, hierarchical namespaces for groups/macros/data, single flat namespace for rule IDs, package distribution model, compile-time conflict detection. - ADR-0015: Macros and Named Compositions — typed expression macros, pure and inlined at compile time, file/group/package scoped. Resolves the "named composition" references scattered across earlier ADRs. - ADR-0016: External Data Sources — typed `data` declarations for IP lists, regex sets, geo databases, regex assemblies. Replaces the implicit `@ipMatchFromFile` style with compile-time-validated references. Also adds docs/TODO.md tracking second-tier and lower-priority structural gaps (durations, versioning, function signatures, test attachment, persistent collections, capture groups) with reserved ADR numbers. Cleanups in existing ADRs to keep cross-references consistent: - README: Status section, Phase 0 reframed (open vs proposed), Phase 3 table fixed (scoring_threshold → config), Pipeline operator framing softened to acknowledge Phase 0 dependency. - ADR-0004: corrected stale claim that ADR-0011 lifts scoring_threshold to globals (it's actually ADR-0012's config block); updated example. - ADR-0007: noted that most cited TX-only patterns are now eliminated by ADR-0006/0011/0012; added note on rule 901001 as compiler-generated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ADRs 0013–0016 (documentation model, imports/modularity, macros, external data), updates README to mark designs as proposed and reassign features across phases, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant FS as FileSystem
participant Compiler as Compiler
participant Remote as RemoteDataSource
participant Engine as TargetEngine
Dev->>Compiler: open / compile `package.crs` (sources + macros + imports)
Compiler->>FS: read files & `import` targets
FS-->>Compiler: file contents
Compiler->>Remote: fetch external `data` (if URL + checksum)
Remote-->>Compiler: data payload / checksum validation
Compiler->>Compiler: parse, validate types, expand macros (inline), resolve imports
Compiler->>Compiler: generate IR (`Documentation`, `DataDecl`, flattened rules)
Compiler->>FS: write outputs (SecLang `.conf`, YAML/manifests)
Compiler->>Engine: emit export / manifest indicating runtime data references
Engine-->>Compiler: runtime load semantics (static vs dynamic) noted in output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0004-structured-actions.md`:
- Line 171: The fenced code block containing "rule 900100 (phase:
request_headers) { ... }" is missing a language identifier which triggers
markdownlint MD040; update the fence to include the language tag (e.g., add
"crslang" after the opening ```), so the block becomes ```crslang followed by
the existing rule code and a closing ```.
In `@docs/adr/0013-documentation-model.md`:
- Line 65: Several fenced code blocks in the ADR "0013-documentation-model" are
missing language identifiers (MD040); update each triple-backtick fence for the
examples (the blocks at the noted positions) to include the appropriate language
tag (e.g., use crslang for CRS examples, yaml for YAML snippets, go for Go code,
or hcl for Terraform) so markdownlint stops warning; search for the unlabeled
``` fences in the document (around the examples referenced) and change them to
```crslang, ```yaml, ```go or ```hcl as appropriate.
In `@docs/adr/0014-imports-and-modularity.md`:
- Line 26: Several fenced code blocks in the "But the proposals do not specify:"
section and the other nearby fenced examples are missing language identifiers
(markdownlint MD040); update each triple-backtick fence (the blocks beginning
near the "But the proposals do not specify:" paragraph and the other example
blocks) to include the appropriate language tag (e.g., ```js, ```ts, ```bash, or
```text as appropriate for the snippet) so each fenced block has a language
identifier and satisfies MD040.
- Around line 124-126: The prose states “last-import-wins on collision, with a
compile-time warning” but the conflict table says the same scenario is a
compile-time error; make these semantics consistent by adopting strict error
semantics: change the prose phrase “last-import-wins on collision, with a
compile-time warning” to “collisions are a compile-time error” and update the
conflict table entry (and any related entries in the same conflict section) to
mark unaliased-import collisions as a compile-time error rather than a warning
so both narrative and table agree.
In `@docs/adr/0016-external-data-sources.md`:
- Line 53: Several fenced code blocks in the ADR titled "0016 - External Data
Sources" are missing language specifiers (MD040); update each triple-backtick
fence used for examples so they include an appropriate language tag (e.g.,
```crslang, ```go, ```yaml) instead of bare ```; prefer specific tags for typed
snippets and ensure all code fences referenced in the review are updated
consistently so linters recognize the language.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5973e847-e2df-4a58-b30f-825e210a68c8
📒 Files selected for processing (8)
docs/README.mddocs/TODO.mddocs/adr/0004-structured-actions.mddocs/adr/0007-phase-inference.mddocs/adr/0013-documentation-model.mddocs/adr/0014-imports-and-modularity.mddocs/adr/0015-macros-and-compositions.mddocs/adr/0016-external-data-sources.md
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new architecture decision records (ADRs) that fill in previously under-specified CRSLang design areas (documentation model, imports/modularity, macros, and external data sources), plus a TODO tracker and cross-reference consistency updates across existing docs.
Changes:
- Add ADRs 0013–0016 covering documentation tiers, import/namespacing semantics, macro design, and typed external data sources.
- Add
docs/TODO.mdto track next-wave structural gaps with reserved ADR numbers. - Update existing docs (
docs/README.md, ADR-0004, ADR-0007) to align examples and references with the newer ADRs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/adr/0013-documentation-model.md | Introduces a 4-tier documentation model (ruleset/group/rule/directive) with narrative doc-comments + structured fields. |
| docs/adr/0014-imports-and-modularity.md | Defines explicit imports, namespace rules, load order, idempotency, and conflicts for multi-file/package composition. |
| docs/adr/0015-macros-and-compositions.md | Specifies typed, pure, compile-time-inlined expression macros with scoping rules. |
| docs/adr/0016-external-data-sources.md | Proposes typed data declarations for external sources (files/URLs/inline) and target-specific compilation mappings. |
| docs/adr/0007-phase-inference.md | Updates narrative/examples to reflect newer proposals that reduce TX-only authored patterns. |
| docs/adr/0004-structured-actions.md | Refreshes examples/notes to reflect config/scoring/paranoia moving out of TX-assignment setup patterns per newer ADRs. |
| docs/TODO.md | Tracks unaddressed structural concepts and reserves ADR numbers for future work. |
| docs/README.md | Adds “Status” framing, updates phase descriptions, and links ADRs 0013–0016 into the roadmap. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
docs/adr/0014-imports-and-modularity.md (1)
12-12:⚠️ Potential issue | 🟡 MinorFix bare fenced blocks to satisfy MD040.
Line 12, Line 59, Line 81, Line 104, Line 171, Line 203, Line 216, Line 229, and Line 242 need fence language tags. Use
textfor these CRSLang/pseudocode/file-tree snippets.Suggested fix pattern
-``` +```text ...</details> Based on learnings: In this repository’s Markdown docs, `crslang` is not a recognized fence language; use `text`/`plaintext` or explicitly suppress MD040. Also applies to: 59-59, 81-81, 104-104, 171-171, 203-203, 216-216, 229-229, 242-242 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/adr/0014-imports-and-modularity.mdat line 12, Several fenced code
blocks in this ADR use bare fences or the nonstandard language tagcrslang,
triggering MD040; update each affected fence to include an explicit language tag
by replacing opening fences likeorcrslang with ```text (orre-run the linter to confirm MD040 is resolved.docs/adr/0016-external-data-sources.md (1)
53-53:⚠️ Potential issue | 🟡 MinorAdd language identifiers to all fenced code blocks (MD040).
Line 53, Line 59, Line 156, Line 167, Line 178, Line 280, and Line 293 still use bare fences, which will keep markdownlint failing. Use
text(orplaintext) for CRSLang snippets in this repo.Suggested fix
-``` +```text data <name> from <source> type <type> [format <format>] [reload <policy>]...
-+textIP list from a file, hot-reloadable
...
Based on learnings: In this repository,
crslangis not a supported fenced-code language tag; usetext/plaintext(or explicitly suppress MD040).Also applies to: 59-59, 156-156, 167-167, 178-178, 280-280, 293-293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0016-external-data-sources.md` at line 53, Update all bare fenced code blocks in the ADR document that contain CRSLang snippets to include a language identifier (use "text" or "plaintext") so markdownlint MD040 passes; specifically modify the fenced blocks referenced around the CRSLang examples (the blocks shown in the diff such as the snippet starting with "data <name> from <source> type <type> ..." and other CRSLang examples) by changing their opening backticks to ```text (or ```plaintext) for each occurrence (lines noted in the review: 53, 59, 156, 167, 178, 280, 293) to ensure consistent tagging or alternatively add an MD040 suppression if you intentionally want bare fences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0016-external-data-sources.md`:
- Around line 78-84: The example uses the token managed_by_engine on the data
declaration (data geo_db) but the declared grammar doesn't accept that token, so
either extend the grammar to accept managed_by_engine on data declarations
(update the parser/IR production that handles data declarations to parse and
preserve a managed_by_engine flag and propagate it into the IR) or remove
managed_by_engine from the examples and instead express “engine-managed” via the
documented metadata fields for the data source; pick one approach and make
consistent changes to the data declaration handling (parser/AST/IR code paths
that consume data declarations) and to all examples (e.g., the data geo_db
example) so the grammar and examples match.
- Around line 116-121: Add a concrete example showing the URL source and its
required checksum: include a snippet that demonstrates using the from "<url>"
form (HTTPS only), the sibling checksum field with the exact `sha256:<hex>`
format, and other commonly used keys (e.g., a resource name like `data
threat_feed`, `type`, `format`, and `reload interval`) so readers see how to
declare `checksum "sha256:<hex>"` alongside `from "<url>"`.
---
Duplicate comments:
In `@docs/adr/0014-imports-and-modularity.md`:
- Line 12: Several fenced code blocks in this ADR use bare fences or the
nonstandard language tag `crslang`, triggering MD040; update each affected fence
to include an explicit language tag by replacing opening fences like ``` or
```crslang with ```text (or ```plaintext) so the snippets
(CRSLang/pseudocode/file-tree) are recognized, and re-run the linter to confirm
MD040 is resolved.
In `@docs/adr/0016-external-data-sources.md`:
- Line 53: Update all bare fenced code blocks in the ADR document that contain
CRSLang snippets to include a language identifier (use "text" or "plaintext") so
markdownlint MD040 passes; specifically modify the fenced blocks referenced
around the CRSLang examples (the blocks shown in the diff such as the snippet
starting with "data <name> from <source> type <type> ..." and other CRSLang
examples) by changing their opening backticks to ```text (or ```plaintext) for
each occurrence (lines noted in the review: 53, 59, 156, 167, 178, 280, 293) to
ensure consistent tagging or alternatively add an MD040 suppression if you
intentionally want bare fences.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6436981-720c-4703-8eaf-4375603ff1fa
📒 Files selected for processing (2)
docs/adr/0014-imports-and-modularity.mddocs/adr/0016-external-data-sources.md
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
ADR-0015 previously said macros were "explicitly exported" without defining the syntax. Pin it down: the `export` keyword prefixes the declaration, applied at file or group scope, with a qualified-path table for what importers see in each case. Default is private (opt-in exports), matching Rust `pub` / TypeScript `export` / Python `__all__` conventions and keeping the public API grep-able. Also covers re-export syntax for surfacing macros declared in other files of the same package, the HCL-surface equivalent (`visibility` attribute), and adds `Exported bool` to MacroDecl in the IR section. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0015-macros-and-compositions.md`:
- Around line 305-307: The doc and IR types use two names for the same
concept—MacroDeclaration and MacroDecl—create a single canonical name (pick one,
e.g., MacroDecl) and use it everywhere: update the ADR text that currently
references MacroDeclaration to the chosen name, rename the IR/type declaration
(if present) from MacroDeclaration to MacroDecl or vice versa, and update any
AST node constructors, type aliases, comments, and tests that reference the
other variant so all code and docs consistently use the same symbol (MacroDecl
or MacroDeclaration).
- Around line 279-284: The nested fenced code blocks (e.g., the snippet for
macro deep_normalize and the HCL blocks) are missing the required blank lines
before and after the triple-backtick fences inside list items, causing MD031
failures; fix by adding a blank line above each opening ``` and a blank line
below each closing ``` for the `macro deep_normalize` block and the HCL examples
(the blocks around the HCL locals and macro "deep_normalize") so each fenced
block is separated from surrounding list text.
- Around line 101-105: There is a contradiction between the file-scope
definition and the later statement about non-exported macros being usable by
other files; pick one semantics and make the text consistent: either keep "File
scope — declared at the top of a file, visible only within that file" and change
the later line that claims non-exported macros are usable by other files so it
instead says non-exported macros are not visible outside the declaring file, or
change the file-scope line to describe a "file-local unless package-exported via
package.crs" behavior and reference ADR-0014; update all occurrences (the bullet
for File scope and the paragraph currently asserting cross-file visibility) to
use the same rule and mention package.crs/ADR-0014 when describing package-level
exporting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68782213-5935-4eec-8e04-603373e8d8f6
📒 Files selected for processing (1)
docs/adr/0015-macros-and-compositions.md
The previous design required `checksum "sha256:<hex>"` on every URL source, which contradicts itself for live feeds: the first reload after upstream updates will fail verification because the pinned hash no longer matches. Pinned checksums only work for content-pinned URLs. Two verification modes now: - `checksum` pins payload bytes — for versioned/immutable URLs, used with `reload static` or `reload on_startup`. Incompatible with live reloads. - `signed_by` pins the signer (local key file) and verifies a detached signature on each fetch. Suitable for live feeds because the signer is pinned, not the content. URL sources require at least one of the two. Both may be present. Examples updated to show a pinned versioned release alongside a signed-and-reloaded threat feed. Risks section reworded to match. Also extends the grammar to accept `managed_by_engine` (used in the geo_db example but previously not in the grammar) and fills in the URL example that was empty after a prior linter pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…radiction
Three review fixes:
- Rename `MacroDeclaration` (one stray reference in the HCL surface
section) to `MacroDecl` to match the canonical name used by the IR
struct definition and elsewhere in the doc.
- Add blank lines around the Custom-parser fence and around both
nested HCL code blocks inside the bullet list, fixing MD031
fenced-code-blocks-surrounded-by-blank-lines violations.
- Resolve the contradiction between the File-scope bullet ("visible
only within that file") and the Exports section ("non-exported
macros are still usable by other files in the same package").
Per ADR-0014, file-scope macros become visible to other files
only via explicit `import` of the declaring file. Updated both
passages to state this consistently, and tightened the
Package-scope bullet to reference the `export` keyword and Exports
section instead of the older "exported by package.crs" wording.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Adds four new ADRs covering structural concepts that were missing or under-specified in the prior set:
importdirective, hierarchical namespaces for groups/macros/data, single flat namespace for rule IDs, package distribution model, compile-time conflict detection.datadeclarations for IP lists, regex sets, geo databases, regex assemblies. Replaces the implicit@ipMatchFromFilestyle with compile-time-validated references.Also adds docs/TODO.md tracking second-tier and lower-priority structural gaps (durations, versioning, function signatures, test attachment, persistent collections, capture groups) with reserved ADR numbers.
Cleanups in existing ADRs to keep cross-references consistent:
Summary by CodeRabbit
Documentation
New Features