Loading…
sources · 1
…ave the origin
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
AI
lite review requested due to automatic review settings
** lukekim
**
previously approved these changes
[Aug 5, 2026](#pullrequestreview-4860820008)
Resolve the conflict in bin/spice/src/context.rs by deferring to the shared redirect policy trunk now provides.
Both sides added a helper next to the same constants: this branch a local same_origin_redirect_policy, trunk a normalize_credential. Trunk also landed spice_cloud_client::redirect::same_origin_redirect_policy, so the local copy became a duplicate -- and the weaker of the two. It compares scheme, host and effective port by hand, which reads two hostless URLs (file:///a and file:///b) as one origin; the shared helper defers to Url::origin, where a scheme with no host gets an opaque origin that never compares equal and so is always refused.
Drop the local copy, import the shared one, and keep normalize_credential. What remains of this branch is the part still unique to it: the CLI's RuntimeContext client is built with that policy, and a client that cannot be built is now an error rather than unwrap_or_default -- a defaulted client would silently carry no policy at all.
The unit test over the removed local helper goes with it (the shared crate covers the origin comparison). In its place are two tests through RuntimeContext::with_args, so the client under test is the one RuntimeContext::new builds: a cross-origin 307 is refused and the off-origin listener is never contacted, and a same-origin redirect is still followed.
Copilot AI
review requested due to automatic review settings
deleted the
fix/12495-same-origin-redirect-policy
branch
pull Bot
pushed a commit
to TheRakeshPurohit/spiceai
that referenced
this pull request
…ave the origin (spiceai#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as spiceai#12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes spiceai#12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
Merged
3 tasks done
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
github-actions Bot
pushed a commit
that referenced
this pull request
…ave the origin (#12503)
The CLI sends its API key as an X-API-Key header on a reqwest client that used the default redirect policy, which follows up to 10 redirects. reqwest sanitises headers when a redirect changes origin, but only the standard set -- Authorization, Cookie, Cookie2, Proxy-Authorization, WWW-Authenticate. A custom header is not in it, so a runtime, proxy or ingress answering with a cross-origin Location had the key forwarded to whatever that named.
Follow same-origin redirects only, comparing scheme, host and effective port so that http://host and http://host:80 remain one origin. A cross-origin Location stops instead, which hands the 3xx back to the caller as a response rather than failing the transport. The policy is consulted before the redirected request goes out, so the credential never reaches the new origin.
Set on the context client, which every request-building call site shares: the get/post/post_json helpers, and commands/nsql/mod.rs, search.rs, chat.rs, trace.rs, acceleration.rs and add.rs, none of which builds its own client.
spice query and spice nsql analyze are NOT covered: they build their own
credentialed client from the pinned spiceai SDK, which pins its own reqwest, so
they cannot inherit this policy. Whether they are exposed depends on which
header the SDK uses for the key -- filed as #12502, since it needs an SDK-side
change either way.
The client build no longer falls back with unwrap_or_default(). That fallback is the wrong shape for this fix: a default reqwest::Client has the default redirect policy, so the one path that silently produced a client would produce the leaking one. RuntimeContext::new already returned Result, so it propagates Error::HttpClientBuild instead.
The depth bound is compared the way Policy::limited compares it -- previous() counts the initial URL, so the bound is exclusive.
Fixes #12495
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](https://github.co/hiddenchars)
[Show hidden characters]({{ revealButtonHref }})