chore(confinement): create UnconfinedTemplate and ConfinedTemplate abstractions - #25938
Conversation
…s in confined template pattern
thomasqueirozb
added
the
no-changelog
github-actions
Bot
added
domain: transforms
…mplate to close the render-without-confinement loophole
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 17f43c9 | Docs | Datadog PR Page | Give us feedback! |
…remove test duplication
pront
left a comment
pront
left a comment
There was a problem hiding this comment.
One idea here: first submit a PR including just the the renaming of Template to UnconfinedTemplate. It should make this one easier to review later.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce16c183d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
thomasqueirozb
changed the title
| docs::warnings = "Rendered paths are confined to `base_dir` (derived from the literal prefix of `path` when unset). See the `base_dir` option." | ||
| ))] | ||
| pub path: Template, | ||
| pub path: UnconfinedTemplate, |
There was a problem hiding this comment.
This should be addressed in a followup, but we should have a file-specific type similar to Template so that grabbing the resolved path is not possible unless confinement is applied
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821f491158
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a063bab69
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
pront
left a comment
pront
left a comment
There was a problem hiding this comment.
Mostly looks good, mostly left nits about comments. I will come back for round 2 after these are addressed.
Co-authored-by: Thomas <thomasqueirozb@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
Summary
Introduces template path-confinement as a first-class, type-enforced security control for sinks, so that a log producer that controls an event field used in a templated destination (file path, URL, Kafka topic, Redis key, tenant ID, …) cannot steer a sink to write outside an operator-authored boundary.
The type model
Three template types make the confinement contract impossible to bypass:
Template— the type sink config structs store. It is serde-able and renders as a plain string in generated schemas, but exposes norendermethod. The only thing a sink can do with it is call `.confine()``.ConfinedTemplate(=Confined<UnconfinedTemplate>) — the render-capable type, obtainable only via.confine(). It is deliberately not deserializable and its fields are module-private, so it can never be constructed (or deserialized) without going through confinement. There is no way to render a configured template without first confining it. The chain is alwaysTemplate -> confine() -> ConfinedTemplate -> render().UnconfinedTemplate— deserializable and renderable, for transforms/sources and the few sinks that intentionally render without a confinement boundary (they render arbitrary attribute values, not routing/destination values).How did you test this PR?
Unit tests cover prefix/URI confinement, traversal and injection bypass attempts, startup rejection, and the opt-out path.
make fmt,make check-clippy, andmake testall pass (3262 tests passed, 13 skipped).Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
NA