Q&A Seeking clarification on confinement changes

[cmellongoempyrean]

Question

There doesn't seem to be any documentation of the rationale or implementation behind the rationale changes and they don't make sense to me so I'm seeking some clarification.

They seem to be targeted at preventing route traversal attacks, but they're applied to many fields which are not routes. It's also not clear that, in a general case, adding a string prefix actually prevents traversal attacks unless accompanied by other mitigations, which would be sink-specific.

The behavior is also not clear. Given the example of the file sink, how exactly does the base_dir prevent traversal? What's the behavior if a string like "../../etc" is passed to the event?

I would have expected sink implementations to attempt to prevent naive injection attacks by using quoting. If that's not something we can rely on and we need to ensure our templates are secure against injection, we need tools beyond just adding a literal prefix, that will let us appropriately escape potentially dangerous characters.

In #25894, it was stated that the recommendation is to have literal prefixes on your loki metadata values. Why is that recommended? What kind of attack is that preventing?

In the cloudwatch sink, which is what I was using when I encountered this issue at upgrade, why isn't the log_group attribute a sufficient confinement base?

Confinement feels like a very blunt instrument with a lot of splash radius, that was applied broadly in a lot of cases that that aren't really justified. I'd like to be wrong about this, and to be able to have some detailed explanations in the documentation.

If there are docs and I didn't find them, I apologize - I searched for "confinement" and the only results were the upgrade guides.

Vector Config

No response

Vector Logs

No response

[pront]

Hi @cmellongoempyrean, thanks for creating a discussion. You're largely right in your understanding.

What we did deliberately was apply confinement uniformly to most templates, rather than tailoring it field by field. We made the judgment call to over-restrict now and relax later. Given that we provide a way to fully preserve existing behavior, and that this also resolved issues reported here, we deemed it was worth it.

For path- and routing-style fields, confinement is a genuine defense against traversal and redirection; for others it's a weaker guarantee, hence we don't consider the current iteration final. The most useful way to move that forward is the following: if you think confinement isn't earning its keep for a specific field, please open an issue with your reasoning for why the threat model doesn't apply there. Since this is all in the open, a proposed relaxation with a rationale is genuinely welcome.

In the meantime, dangerously_allow_unconfined_template_resolution: true fully preserves pre-existing behavior. The name is loud on purpose, but it isn't a verdict that every use is reckless. That’s a decision for the person/team who deploys Vector.

Edit: Release page was updated based on the above.

[cmellongoempyrean]

Thanks for the response, and I apologize if my question came off as aggressive, I'm really not trying to second-guess the design decisions, just to make sure that I understand them so I'm confident I'm using things correctly and safely. I will open issues as suggested.

[pront]

You are very welcome. I think at least one of the cases you mentioned above can be relaxed. Please create dedicated issues so we can track each case separately.