feat: endpoint API supports lf-endpoint-version header#3648
feat: endpoint API supports lf-endpoint-version header#3648msmithstubbs wants to merge 3 commits into
Conversation
ed7d994 to
34893b4
Compare
| {:via, :syn, {partition, {query_id, param_hash}}} | ||
|
|
||
| {key, partition_key} = | ||
| case version_number do |
There was a problem hiding this comment.
Caching on version number does mean results for the latest endpoint version can be cached twice: once with the version number, and once without.
There was a problem hiding this comment.
I think to avoid ambiguity, we should always set the version number. If no version created yet for existing endpoints then 0 or nil will work.
There was a problem hiding this comment.
It also means that we essentially can remove cache invalidation on updates as the version number will always change so we don't need to be worried of config drift.
There was a problem hiding this comment.
We would still have to invalidate the cache on update, right?
If I'm requesting an endpoint without specifying a version I'll get cached version 0. If the endpoint is updated the default endpoint served will still be version 0 without the updated config.
There was a problem hiding this comment.
Updated to use :latest as the version number when no version is specified. Think this is clearer. That work?
34893b4 to
d714c2f
Compare
| {:via, :syn, {partition, {query_id, param_hash}}} | ||
|
|
||
| {key, partition_key} = | ||
| case version_number do |
There was a problem hiding this comment.
I think to avoid ambiguity, we should always set the version number. If no version created yet for existing endpoints then 0 or nil will work.
| {:via, :syn, {partition, {query_id, param_hash}}} | ||
|
|
||
| {key, partition_key} = | ||
| case version_number do |
There was a problem hiding this comment.
It also means that we essentially can remove cache invalidation on updates as the version number will always change so we don't need to be worried of config drift.
d714c2f to
92932e4
Compare
92932e4 to
ade0c6f
Compare
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| {:ok, version_number} <- parse_endpoint_version(version_str), | ||
| {:ok, endpoint_query} <- | ||
| Endpoints.get_endpoint_query_at_version(endpoint, version_number) do | ||
| assign(conn, :endpoint_query, endpoint_query) |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
The versioned endpoint_query loaded here preserves sandboxable: true from the historical snapshot. If a previous version had sandboxable: true and the owner later disabled it, any caller can use LF-ENDPOINT-VERSION: <old> to re-enable arbitrary SQL execution via the sql/lql params, bypassing the owner's security intent.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: After loading the versioned endpoint_query, override its sandboxable field with the value from the current live endpoint (already in scope from the function head pattern match). This ensures that if the owner has since disabled sandboxing, the historical snapshot cannot be used to re-enable the custom SQL/LQL execution path. Replace the bare assign/3 call with one that explicitly pins sandboxable to the current endpoint's value.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| assign(conn, :endpoint_query, endpoint_query) | |
| assign(conn, :endpoint_query, %{endpoint_query | sandboxable: endpoint.sandboxable}) |
There was a problem hiding this comment.
This is true, but I'm not sure if it falls within the intent of this feature. Please advise, @Ziinc
2 participants
Adds version selection to the endpoint API with the
lf-endpoint-versionheaderendpoint_versionlabellf-endpoint-versionheader is set the current version of the endpoint query is used.Fixes O11Y-1064