You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
data-plane: ask the API once for a Native files request with a small body - #1611
A Native files:* request with a small body now costs the data plane one call to the lifecycle API instead of two, with the same authorization on every request and nothing cached.
The data plane asked the API's activation listener twice per request: AuthorizeNativeFileRequest before reading the body, then PrepareNativeFileCall after checking it. Each call runs the request through the public route's router, so the API checked the credential and the sandbox's owner twice, and the request paid two round trips before its sandbox call. PrepareNativeFileCall already authorizes the request itself before it binds a key, charges admission or wakes the sandbox, so the first call only fixed the order of answers.
Now a request is checked before the API is asked when it is a POST that declares a body of at most 16 KiB and does not send Expect: 100-continue. Every body except a write's is one or two paths and a few fields. If its checks pass, the data plane asks only PrepareNativeFileCall. If they fail, it asks AuthorizeNativeFileRequest and answers with the API's refusal when there is one, so an unauthenticated request with a bad body still gets the 401 the API's own route gives, not a 400. A write, a request with no declared length or a larger one, and one waiting for 100 Continue take the existing path: authorized from its headers before its body is read, then admitted, checked and prepared.
The gRPC channel to the activation listener and the channels to sandbox runtimes were already shared and reused across requests; this change does not touch them.
docs/architecture.md and the comments in proto/node/api/v1/native_file_access.proto describe the new order. The RPCs and their messages are unchanged.
Threat analysis
What stays the same:
Every request is authorized by the API, on that request, through the same router: credential, owner scope, federated grant. PrepareNativeFileCall runs that before it binds an Idempotency-Key, charges admission or wakes the sandbox, so an unauthorized request has no side effect. Nothing is cached, so a revoked key, a deleted sandbox or a suspended one is answered on its next request exactly as before.
Answers: a request that fails its checks is authorized first and gets the API's refusal if it has one; one that passes gets PrepareNativeFileCall's refusal, which the same router renders in the same envelope as AuthorizeNativeFileRequest's. A refused request is never sent to the sandbox.
API load per refused request: one RPC either way.
What changes, for a client with no valid credential:
It can make a replica read and parse up to 16 KiB of body before it is refused, where it was refused after its headers. That is less than the headers the data plane already reads and sends to the API for authorization (the API accepts up to 256 KiB of them), and a refusal still costs the API one call.
A client that sends a declared body of at most 16 KiB slowly holds its request until the body arrives or the client gives up, since the data plane sets no body read timeout, where it was refused once its headers arrived. The same client could already hold a request open by sending its headers slowly.
Large or undeclared bodies, writes, and Expect: 100-continue requests keep the previous order, so a client can't make a replica buffer more without authenticating first.
A short cache of authorization results was considered and not taken. After this change the only call left is the one that returns the sandbox's current runtime and route token and wakes the sandbox, which the data plane would need on every request anyway. Caching authorization would save nothing more without also caching routing and wake state, and it would delay revocation, which docs/architecture.md says takes effect on the next request.
Validation
reads_answer_through_the_granted_route_in_the_apis_shapes now expects one prepare per read and fails without the change (it sees authorize, prepare for each).
a_small_body_is_checked_before_the_api_is_asked_and_its_refusal_still_comes_first fails without the change, and with FileRequest::checked not asking the API (a bad body then answers 400 to an unauthenticated request).
a_body_too_large_to_check_first_is_authorized_before_it_is_read fails with the 16 KiB bound removed, and with the Expect check removed.
cargo fmt, cargo clippy -D warnings on pplx-space-data-plane-server, cargo check --workspace --all-targets --locked, and cargo nextest for pplx-space-data-plane-server (419 passed): all pass. cargo arch passes.
Kind, one sandbox each on the current seed guest, a 4 KiB files:read over a kept-alive connection from the host, 60 samples after warm-up:
p50 / p90
main
this change
Native on the data plane
5.5 / 5.8 ms
4.6 / 5.0 ms, and 4.7 / 5.5 ms in a second run
Native on the API
4.3 / 5.5 ms
4.1 / 4.4 ms
ECW GET /files on the data plane
3.1 / 3.3 ms
3.2 / 3.4 ms
The data plane's lead over the API's own route went from 1.2 ms to 0.5 ms at the median. On Kind the API and its database share one host, so each call saved costs less than on a deployment.
On Kind, files:stat sent to the API and to the data plane got the same status, code and message for a bad key with a malformed body (401), a bad key with a valid body (401), a valid key with a malformed body (400), and a valid key naming a missing sandbox (404).
Native typed smoke on Kind with SPACE_SMOKE_KIND_ADMIN_TOKEN_FILE set: 47 passed, 12 ignored (opt-in bench scenarios), none failed, including the data-plane files scenarios (operations match on the data plane, idempotency across origins and conflicts, wake of a suspended sandbox, upload admission, write size boundary).
Severity and risk
Severity: low to medium. Native file calls served by the data plane were slower than the same calls on the API, which they exist to offload.
Risk: a change to what the data plane does before a request is authorized. The threat analysis above lists what an unauthenticated client gains. A request whose body is invalid now costs the data plane a body read before its refusal. Roll back by reverting; nothing is stored, and the API is unchanged, so the data plane can be rolled back alone.
Rollout
The API is unchanged, and PrepareNativeFileCall has always authorized the request itself, so a new data plane works with any API that serves these RPCs.
Not covered
Writes keep two calls: their admission needs the org the API authorizes before the body is read, and must refuse before the key is bound or the sandbox woken.
ECW file calls stay faster: the ECW routes resolve the sandbox's route from a cache keyed by the credential, which Native files do not use.
Not measured on a deployed cluster.
AI assistance: Claude Code drafted the code, tests, and this description; the author reviewed them.
…body
The data plane asked the API's activation listener twice for every Native
`files:*` request: AuthorizeNativeFileRequest before reading the body, then
PrepareNativeFileCall after checking it. PrepareNativeFileCall already
authorizes the request through the same router before it binds a key,
charges admission or wakes the sandbox, so the first call only fixed the
order of answers, at the cost of a round trip and a second credential and
owner check.
A POST that declares a body of at most 16 KiB, and does not wait for
`100 Continue`, is now checked before the API is asked. If its checks pass,
only PrepareNativeFileCall is asked. If they fail, the data plane asks
AuthorizeNativeFileRequest and answers with the API's refusal when there is
one, so a request is still answered as the API's own route answers it.
Writes, requests that declare no length or more, and requests waiting for
`100 Continue` keep the previous order: authorized from their headers before
their body is read. Nothing is cached, so a revoked credential is still
refused on its next request.
Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.
Commit: 8e383cc65abe.
No actionable comments were generated in the recent review.
Walkthrough
Eligible small POST requests can undergo body checks before API authorization. Other requests authorize before body reads. Failed checks on the deferred authorization path trigger authorization before the refusal is returned. Read and mutation handlers use the checked request flow, and write admission uses the authorized identity. Tests and documentation reflect the request ordering and API call changes.
Possibly related PRs
perplexityai/SPACE#1373: Adds the mutation handlers whose body-check and authorization order changes here.
perplexityai/SPACE#1379: Documents Native file authorization and preparation behavior that this PR updates.
Merge Risk: High · up to 8e383
Small file requests are now checked before the API authorizes them, and for requests that pass, authorization happens only inside the prepare call. The change has tests and no concrete defect was found. Because it changes where authorization happens in the native files API, it needs careful owner sign-off before merge.
Security architecture risk: Moderate · up to 8e383
Small file requests can now occupy request-handling resources before credentials are checked. The body-size cap limits bytes, but no body-read deadline has been established for a slow sender. File access remains protected by authorization; the material concern is shared-server availability, with production ingress limits still unverified.
Retained concerns
Medium · security · inferred: The new early-check path lets unauthenticated slow senders retain body-reading handlers and HTTP connections or streams before authorization. The 16 KiB eligibility cap bounds body bytes, not duration or aggregate resource consumption. This expands exposure relative to the base, which authorized these requests before reading their bodies; effective exhaustion limits remain dependent on unverified ingress controls.
Security review details
Security Blast Radius
inferred — The independently attackable resource is data-plane ingress capacity, not an authorized tenant's filesystem. A client does not need valid credentials or sandbox ownership to reach eligible body parsing. Repeated slow requests could pressure a reachable replica and colocated HTTP routes; fleet-wide scope and practical concurrency are not established without deployment limits.
Security Findings and Attack Paths
inferred — The retained denial-of-service finding is consistent with an unauthenticated client sending an eligible JSON POST and withholding body completion. The handler waits in body extraction before authorization can refuse the request. The missing application body deadline is pre-existing, but this pre-authorization waiting path is introduced by the PR; service-exhaustion impact remains conditional on effective ingress limits.
Trust Boundaries and Controls
observed — Early work is limited to local request checks: it does not reserve upload capacity, bind idempotency keys, wake a sandbox or obtain a runtime grant. Writes still authorize before admission. Failed early checks authorize before returning, and successful checks must pass API preparation, so the traced change does not bypass credential or sandbox-owner enforcement.
Resilience and Maintainability Implications
observed — The inspected controls do not establish a normal-operation body-read deadline. TCP keepalive detects dead peers but leaves responsive peers connected; traffic middleware delegates body polling without adding a deadline. Drain stops new work and can terminate outstanding work after a configured shutdown grace, but that is a shutdown control rather than a bound on live slow-body requests.
Hardening Proposals
proposed — Bound both the duration and aggregate concurrency of pre-authorization body reads, or retain header authorization first until equivalent deployed ingress controls are demonstrated. Verify that any added limits release resources on interruption and preserve the intended authorization-refusal semantics.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title follows the required format and describes the main change to small-body Native files requests.
Description Follows The Pr Template
The description contains all four required items: Summary, Validation, Severity and risk, and an AI assistance line. Each has content.
Bug Fixes Name A Regression Test
The Validation section names reads_answer_through_the_granted_route_in_the_apis_shapes and states that it fails without the change and passes with it. The test now asserts one prepare call per rea…
Weakened Safeguards Are Justified
The changed assertions reflect the documented reduction from two API calls to one. The diff adds coverage for authorization order and refusals. It does not remove or weaken an unjustified safeguard.
Public-Safe Content
The diff, title, and description contain no identified prohibited content. The referenced test credential is a fake. The reported measurements are from Kind, not production.
Full details: Bug Fixes Name A Regression Test
Explanation
The Validation section names reads_answer_through_the_granted_route_in_the_apis_shapes and states that it fails without the change and passes with it. The test now asserts one prepare call per read request.
Recent review info
Automatic approval policy
Commit: 8e383cc65abe.
Setting
Effective value
Source
Automatic approvals
Disabled
Override 4
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 2 remain after this review.
This branch has not been deployed
No deployments
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
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.
Summary
A Native
files:*request with a small body now costs the data plane one call to the lifecycle API instead of two, with the same authorization on every request and nothing cached.The data plane asked the API's activation listener twice per request:
AuthorizeNativeFileRequestbefore reading the body, thenPrepareNativeFileCallafter checking it. Each call runs the request through the public route's router, so the API checked the credential and the sandbox's owner twice, and the request paid two round trips before its sandbox call.PrepareNativeFileCallalready authorizes the request itself before it binds a key, charges admission or wakes the sandbox, so the first call only fixed the order of answers.Now a request is checked before the API is asked when it is a POST that declares a body of at most 16 KiB and does not send
Expect: 100-continue. Every body except a write's is one or two paths and a few fields. If its checks pass, the data plane asks onlyPrepareNativeFileCall. If they fail, it asksAuthorizeNativeFileRequestand answers with the API's refusal when there is one, so an unauthenticated request with a bad body still gets the 401 the API's own route gives, not a 400. A write, a request with no declared length or a larger one, and one waiting for100 Continuetake the existing path: authorized from its headers before its body is read, then admitted, checked and prepared.The gRPC channel to the activation listener and the channels to sandbox runtimes were already shared and reused across requests; this change does not touch them.
docs/architecture.mdand the comments inproto/node/api/v1/native_file_access.protodescribe the new order. The RPCs and their messages are unchanged.Threat analysis
What stays the same:
PrepareNativeFileCallruns that before it binds anIdempotency-Key, charges admission or wakes the sandbox, so an unauthorized request has no side effect. Nothing is cached, so a revoked key, a deleted sandbox or a suspended one is answered on its next request exactly as before.PrepareNativeFileCall's refusal, which the same router renders in the same envelope asAuthorizeNativeFileRequest's. A refused request is never sent to the sandbox.What changes, for a client with no valid credential:
Expect: 100-continuerequests keep the previous order, so a client can't make a replica buffer more without authenticating first.A short cache of authorization results was considered and not taken. After this change the only call left is the one that returns the sandbox's current runtime and route token and wakes the sandbox, which the data plane would need on every request anyway. Caching authorization would save nothing more without also caching routing and wake state, and it would delay revocation, which
docs/architecture.mdsays takes effect on the next request.Validation
reads_answer_through_the_granted_route_in_the_apis_shapesnow expects oneprepareper read and fails without the change (it seesauthorize,preparefor each).a_small_body_is_checked_before_the_api_is_asked_and_its_refusal_still_comes_firstfails without the change, and withFileRequest::checkednot asking the API (a bad body then answers 400 to an unauthenticated request).a_body_too_large_to_check_first_is_authorized_before_it_is_readfails with the 16 KiB bound removed, and with theExpectcheck removed.cargo fmt,cargo clippy -D warningson pplx-space-data-plane-server,cargo check --workspace --all-targets --locked, andcargo nextestfor pplx-space-data-plane-server (419 passed): all pass.cargo archpasses.files:readover a kept-alive connection from the host, 60 samples after warm-up:GET /fileson the data planeThe data plane's lead over the API's own route went from 1.2 ms to 0.5 ms at the median. On Kind the API and its database share one host, so each call saved costs less than on a deployment.
files:statsent to the API and to the data plane got the same status, code and message for a bad key with a malformed body (401), a bad key with a valid body (401), a valid key with a malformed body (400), and a valid key naming a missing sandbox (404).SPACE_SMOKE_KIND_ADMIN_TOKEN_FILEset: 47 passed, 12 ignored (opt-in bench scenarios), none failed, including the data-plane files scenarios (operations match on the data plane, idempotency across origins and conflicts, wake of a suspended sandbox, upload admission, write size boundary).Severity and risk
Severity: low to medium. Native file calls served by the data plane were slower than the same calls on the API, which they exist to offload.
Risk: a change to what the data plane does before a request is authorized. The threat analysis above lists what an unauthenticated client gains. A request whose body is invalid now costs the data plane a body read before its refusal. Roll back by reverting; nothing is stored, and the API is unchanged, so the data plane can be rolled back alone.
Rollout
The API is unchanged, and
PrepareNativeFileCallhas always authorized the request itself, so a new data plane works with any API that serves these RPCs.Not covered
AI assistance: Claude Code drafted the code, tests, and this description; the author reviewed them.