docs(appsec): add AppSec developer reference docs and module AGENTS.md files#11454
docs(appsec): add AppSec developer reference docs and module AGENTS.md files#11454jandro996 wants to merge 1 commit into
Conversation
…d files Adds docs/appsec/ with six reference documents covering blocking patterns, WAF API, IG event wiring, file content reading, RASP CSI, and multipart framework invariants. Adds lean AGENTS.md files in netty-4.1, tomcat-appsec, akka-http, jetty-appsec, vertx-web, undertow, and the appsec module pointing to the extended docs.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master resultsStartup Time
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
jandro996
added
type: documentation
tag: no release notes
smola
left a comment
smola
left a comment
There was a problem hiding this comment.
A few comments. A lot of things here do need documentation but:
- We should avoid large snippets of code copied to agent guidance. They'll go out of sync. Just use paths to the relevant files.
- A lot of guidance very specific to a single instrumentation or class is better handled by comments in the relevant code, not as general guidance. When you're not working in that file, they are noise. And you risk missing the doc when you're actually working in the file.
- Some things should be tests, not agent guidance.
|
|
||
| ## Adding a new WAF address: 4-file checklist | ||
|
|
||
| Every new WAF address requires changes in exactly 4 files. Missing any one causes a silent no-op: |
There was a problem hiding this comment.
Missing any one causes a silent no-op -> Maybe we could add a test that checks all these 4 files are coherent in their registered addresses?
Then we wouldn't need to document it at all. If you miss them, you get a test error explaining the issue.
| } | ||
| ``` | ||
|
|
||
| The ID must be unique and sequential. Check the last used ID before assigning. |
There was a problem hiding this comment.
Could we add a test for this instead?
| ### 1. `Events.java` — declare the event type | ||
|
|
||
| ```java | ||
| static final int REQUEST_FILES_CONTENT_ID = 31; // unique, increment from last used |
There was a problem hiding this comment.
This snippet will easily go out of date. I think agents will pick up the pattern just correctly if you say here that the event type must be declared in Events.java (maybe use full path).
| new Address<>("server.request.body.files_content"); | ||
| ``` | ||
|
|
||
| Also add it to the `fromString()` switch. |
There was a problem hiding this comment.
Can we add a test that verifies this is done correctly, instead of documenting it?
| All handlers follow the same retry-on-expiry pattern: | ||
|
|
||
| ```java | ||
| private Flow<Void> onRequestFilesContent(RequestContext ctx_, List<String> filesContent) { |
There was a problem hiding this comment.
Ideally we shouldn't have this big snippet in the docs. It'll get out of sync. I'm sure agents will pick up the pattern from the surrounding context.
| @@ -0,0 +1,168 @@ | |||
| # Multipart AppSec — Framework-Specific Patterns | |||
There was a problem hiding this comment.
All or most of this should be comments right in the instrumentation code or closer to the relevant code.
|
|
||
| ## Do not extract filenames callback dispatch into `UnmarshallerHelpers` | ||
|
|
||
| Extracting the `requestFilesFilenames` callback dispatch into a shared helper method in |
There was a problem hiding this comment.
Which problems does it cause? Unless more detail is given, this risks becoming cargo cult and nobody remembers why we do it.
|
|
||
| ## Jetty 8.x: no `getSubmittedFileName()`, manual parsing required | ||
|
|
||
| `Part.getSubmittedFileName()` (Servlet 3.1) is not available in Jetty 8.x (Servlet 3.0). Filenames |
There was a problem hiding this comment.
Better documneted in the code, not here.
| @@ -0,0 +1,286 @@ | |||
| # WAF API Reference — libddwaf-java | |||
|
|
|||
| Reference for the `libddwaf-java` binding used by the AppSec module. Based on libddwaf-java v1.30.0 / libddwaf v1.30.0. | |||
There was a problem hiding this comment.
Link github.com/datadog/libddwaf-java here. I'll help agents accessing its source code.
| ``` | ||
|
|
||
| **Values used in dd-trace-java** (see `AppSecSystem` / `WafInitialization`): | ||
| - `maxDepth = 20` |
There was a problem hiding this comment.
Let's not copy values here, it'll go out of sync. Referencing the path to the relevant files is enough.
2 participants
Summary
docs/appsec/with six reference documents: blocking patterns per framework, WAF API reference, IG event wiring checklist, multipart file content reading, RASP CSI pattern, and multipart framework invariants (Jersey/RESTEasy/Jetty/GlassFish)AGENTS.mdfiles innetty-4.1,tomcat-appsec,akka-http,jetty-appsec,vertx-web,undertow, and theappsecmodule - each pointing to the extended docsAGENTS.mdwith an AppSec entry in the index tableNo functional code changes.