starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-26 08:01:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(apps): honor presigned S3 signature on app display/preview routes (#10141)

Browse Source 
The app provenance gate short-circuits on a valid presigned signature, but only the raw download_s3_file route parsed it. The parquet/csv/table-count/file-preview/metadata routes discarded sig/exp and always fell through to the provenance gate, so a presigned S3 object rendered as a table showed "File restricted" for any viewer who did not produce it. Thread sig/exp through every apps_u S3 display route and forward the presigned bearer from ParqetCsvTableRenderer/DisplayResult.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-07-15 21:18:15 +02:00
committed by GitHub
parent 2092155191
commit 8c725d9e44
7 changed files with 214 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/Cargo.lock
Generated
+2
View File
@@ -13885,6 +13885,8 @@ dependencies = [
"futures",
"gethostname",
"git-version",
"hex",
"hmac",
"lazy_static",
"once_cell",
"opentelemetry 0.30.0",
backend/Cargo.toml
+2
View File
@@ -326,6 +326,8 @@ async-nats.workspace = true
aws-sdk-sqs.workspace = true
aws-config.workspace = true
aws-credential-types.workspace = true
hmac.workspace = true
hex.workspace = true
[workspace.dependencies]
backend/tests/app_s3_onbehalf.rs
+125
View File
@@ -144,6 +144,131 @@ async fn test_deployed_app_s3_onbehalf_provenance(db: Pool<Postgres>) -> anyhow:
Ok(())
}
/// Mint a presigned bearer (`exp=..&sig=..`) exactly as `sign_s3_objects` does:
/// `HMAC-SHA256(workspace_key, "file_key={s3}&exp={exp}")` (no storage param, since
/// these routes send none). `validate_s3_signature` is `private`-gated, so this test
/// only runs with the `private` feature.
#[cfg(feature = "private")]
fn mint_presigned(workspace_key: &str, s3: &str, exp: i64) -> String {
use hmac::{Hmac, Mac};
use sha2::Sha256;
let mut mac = Hmac::<Sha256>::new_from_slice(workspace_key.as_bytes()).unwrap();
mac.update(format!("file_key={s3}&exp={exp}").as_bytes());
let sig = hex::encode(mac.finalize().into_bytes());
format!("exp={exp}&sig={sig}")
}
/// A presigned S3 object (bearer minted by `signS3Objects`) bypasses the provenance
/// gate on EVERY app-scoped display route, not just the raw `download_s3_file`
/// download: a valid signature clears the gate on preview/count/metadata/csv routes,
/// while an unsigned key stays denied and a forged/expired signature is rejected.
#[cfg(feature = "private")]
#[sqlx::test(fixtures("base"))]
async fn test_deployed_app_s3_presigned_bypasses_gate(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let ws = format!("http://localhost:{port}/api/w/test-workspace");
let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
.json(&json!({
"path": APP,
"summary": "s3 presigned test",
"value": {},
"policy": {
"execution_mode": "anonymous",
"triggerables": {},
"allowed_s3_keys": [{ "s3_path": DECLARED }]
}
}))
.send()
.await?;
assert_eq!(resp.status(), 201, "app create: {}", resp.text().await?);
let workspace_key: String = sqlx::query_scalar(
"SELECT key FROM workspace_key WHERE workspace_id = 'test-workspace' AND kind = 'cloud'",
)
.fetch_one(&db)
.await?;
let exp = chrono::Utc::now().timestamp() + 3600;
let presigned = mint_presigned(&workspace_key, NON_PROVENANCE, exp);
let get = |route: String, token: &'static str| {
let url = format!("{ws}/apps_u/{route}");
authed(client().get(url), token).send()
};
let denied = |body: &str| body.contains("File restricted");
// Control: NON_PROVENANCE without a signature is denied by the gate.
let body = get(
format!("download_s3_file/{APP}?s3={NON_PROVENANCE}"),
USER_TOKEN,
)
.await?
.text()
.await?;
assert!(
denied(&body),
"unsigned non-provenance key must be denied: {body}"
);
// Every display route: a valid presigned key clears the gate (falls through to
// the storage read, which fails with a storage error, never "File restricted").
// `read_bytes_*` are required on load_file_preview.
let routes = [
format!("download_s3_file/{APP}?s3={NON_PROVENANCE}&{presigned}"),
format!("load_table_count/{APP}?file_key={NON_PROVENANCE}&{presigned}"),
format!("load_csv_preview/{APP}?file_key={NON_PROVENANCE}&limit=5&offset=0&{presigned}"),
format!("load_parquet_preview/{APP}?file_key={NON_PROVENANCE}&limit=5&offset=0&{presigned}"),
format!("load_file_metadata/{APP}?file_key={NON_PROVENANCE}&{presigned}"),
format!(
"load_file_preview/{APP}?file_key={NON_PROVENANCE}&read_bytes_from=0&read_bytes_length=4096&{presigned}"
),
format!("download_s3_parquet_file_as_csv/{APP}?file_key={NON_PROVENANCE}&{presigned}"),
];
for route in routes {
let body = get(route.clone(), USER_TOKEN).await?.text().await?;
assert!(
!denied(&body),
"presigned key must bypass the gate on {route}: {body}"
);
}
// A tampered signature must NOT bypass: presence of `sig` commits to validation,
// so a wrong sig is rejected outright ("Invalid signature") rather than falling
// back to the provenance gate.
let forged = format!("exp={exp}&sig={}", "00".repeat(32));
let body = get(
format!("download_s3_file/{APP}?s3={NON_PROVENANCE}&{forged}"),
USER_TOKEN,
)
.await?
.text()
.await?;
assert!(
body.contains("Invalid signature"),
"forged signature must be rejected: {body}"
);
// An expired-but-valid signature is rejected on expiry, not bypassed.
let past = chrono::Utc::now().timestamp() - 10;
let expired = mint_presigned(&workspace_key, NON_PROVENANCE, past);
let body = get(
format!("download_s3_file/{APP}?s3={NON_PROVENANCE}&{expired}"),
USER_TOKEN,
)
.await?
.text()
.await?;
assert!(
body.contains("Signature expired"),
"expired signature must be rejected: {body}"
);
Ok(())
}
/// Seed a completed job whose result carries an s3 object. `app_trigger` sets the
/// app-origination marker exactly as `execute_component` stamps it: `Some(app_path)`
/// => `trigger_kind = 'app'` + `trigger = <app_path>` (an app-launched run);
backend/windmill-api/openapi.yaml
+24
View File
@@ -12139,6 +12139,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: FileMetadata
@@ -12191,6 +12193,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: FilePreview
@@ -12241,6 +12245,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: Parquet Preview
@@ -12293,6 +12299,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: Csv Preview
@@ -12325,6 +12333,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: Table count
@@ -12354,6 +12364,8 @@ paths:
in: query
schema:
type: string
- $ref: "#/components/parameters/S3Sig"
- $ref: "#/components/parameters/S3Exp"
responses:
"200":
description: The downloaded file
@@ -22660,6 +22672,18 @@ components:
required: true
schema:
type: string
S3Sig:
name: sig
in: query
description: HMAC signature of a presigned S3 object (bypasses the app provenance gate)
schema:
type: string
S3Exp:
name: exp
in: query
description: Expiry timestamp of a presigned S3 object signature
schema:
type: string
CustomPath:
name: custom_path
in: path
backend/windmill-api/src/apps.rs
+34 -15
View File
@@ -3896,6 +3896,10 @@ async fn check_if_allowed_to_access_s3_file_from_app(
windmill_api_auth::scopes::has_app_embed_sentinel(authed.scopes.as_deref())
});
// A valid presigned bearer is a self-authorizing capability, so it short-circuits
// the provenance gate. OSS builds cannot validate signatures (no workspace-key
// HMAC), so there the bearer is ignored and the request falls through to the
// checks below — the same path these routes took before presigning.
if file_query.sig.is_some() {
#[cfg(feature = "private")]
{
@@ -3908,13 +3912,11 @@ async fn check_if_allowed_to_access_s3_file_from_app(
&db,
)
.await?;
Ok(())
return Ok(());
}
#[cfg(not(feature = "private"))]
return Err(Error::InternalErr(
"Internal error: signature validation is not supported in open source mode".to_string(),
));
} else if matches!(policy.execution_mode, ExecutionMode::Viewer) && !is_app_embed {
}
if matches!(policy.execution_mode, ExecutionMode::Viewer) && !is_app_embed {
// Viewer mode: the on-behalf identity IS the viewer, so the downstream
// get_workspace_s3_resource_and_check_paths already bounds the read by
// their own perms — no provenance gate (it would over-restrict). Embed
@@ -4051,14 +4053,25 @@ async fn download_s3_file_from_app(
.await
}
// Presigned bearer params (`exp=..&sig=..`) extracted as a second `Query` so the
// app-scoped preview/count/metadata routes honor a presigned key the same way the
// raw `download_s3_file` route does.
#[cfg(feature = "parquet")]
fn app_s3_file_query(s3: String, storage: Option<String>) -> AppS3FileQuery {
#[derive(Deserialize)]
struct AppS3Sig {
sig: Option<String>,
#[cfg(feature = "private")]
exp: Option<String>,
}
#[cfg(feature = "parquet")]
fn app_s3_file_query(s3: String, storage: Option<String>, sig: AppS3Sig) -> AppS3FileQuery {
AppS3FileQuery {
s3,
storage,
sig: None,
sig: sig.sig,
#[cfg(feature = "private")]
exp: None,
exp: sig.exp,
}
}
@@ -4154,9 +4167,10 @@ async fn app_download_s3_parquet_file_as_csv(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<DownloadFileQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone());
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
crate::job_helpers_oss::download_s3_parquet_file_as_csv_internal(
@@ -4179,9 +4193,10 @@ async fn app_load_file_metadata(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<LoadFileMetadataQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone());
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
let resp =
@@ -4195,9 +4210,10 @@ async fn app_load_file_preview(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<LoadFilePreviewQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone());
let file_query = app_s3_file_query(query.file_key.clone(), query.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
let resp =
@@ -4211,10 +4227,11 @@ async fn app_load_table_count(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<AppLoadCountQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let (file_key, inner) = query.into_inner();
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone());
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
let resp =
@@ -4229,10 +4246,11 @@ async fn app_load_parquet_preview(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<AppLoadPreviewQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let (file_key, inner) = query.into_inner();
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone());
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
let resp = crate::job_helpers_oss::load_preview_internal(
@@ -4248,10 +4266,11 @@ async fn app_load_csv_preview(
Extension(db): Extension<DB>,
Path((w_id, path)): Path<(String, StripPath)>,
Query(query): Query<AppLoadPreviewQuery>,
Query(sig): Query<AppS3Sig>,
) -> Result<Response> {
let path = path.to_path();
let (file_key, inner) = query.into_inner();
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone());
let file_query = app_s3_file_query(file_key.clone(), inner.storage.clone(), sig);
let job_authed =
app_s3_on_behalf_and_provenance(&db, &path, &w_id, &opt_authed, &file_query).await?;
let resp = crate::job_helpers_oss::load_preview_internal(
frontend/src/lib/components/DisplayResult.svelte
+2
View File
@@ -1060,6 +1060,7 @@
{appPath}
s3resource={s3object?.s3}
storage={s3object?.storage}
presigned={s3object?.presigned}
/>
{/key}
{:else if s3object?.s3?.endsWith('.png') || s3object?.s3?.endsWith('.jpeg') || s3object?.s3?.endsWith('.jpg') || s3object?.s3?.endsWith('.webp')}
@@ -1116,6 +1117,7 @@
{appPath}
s3resource={s3object?.s3}
storage={s3object?.storage}
presigned={s3object?.presigned}
/>{:else}
<button
class="text-primary whitespace-nowrap flex gap-2 items-center"
frontend/src/lib/components/ParqetCsvTableRenderer.svelte
+25 -4
View File
@@ -26,6 +26,10 @@
// through the app-scoped, provenance-gated `apps_u/*` endpoints instead of
// the viewer-scoped `job_helpers/*` API. Undefined in the editor/preview.
appPath?: string | undefined
// HMAC bearer (`exp=..&sig=..`) minted by `signS3Objects`. When present on a
// deployed-app read, it bypasses the provenance gate — matching the download
// and image routes. Forwarded to every app-scoped preview/count/export call.
presigned?: string | undefined
}
let {
@@ -33,9 +37,18 @@
storage,
workspaceId,
disable_download = false,
appPath = undefined
appPath = undefined,
presigned = undefined
}: Props = $props()
// Split the presigned bearer into typed query params for the generated client.
// Only meaningful in deployed-app (`appPath`) reads; empty otherwise.
function presignedParams(): { sig?: string; exp?: string } {
if (!appPath || !presigned) return {}
const p = new URLSearchParams(presigned)
return { sig: p.get('sig') ?? undefined, exp: p.get('exp') ?? undefined }
}
// Route the parquet/csv read through the app-scoped endpoints when `appPath`
// is set, else the viewer-scoped helpers. Same request/response shape either
// way — the only difference is which identity authorizes the S3 read.
@@ -48,7 +61,8 @@
fileKey: s3resource,
searchCol,
searchTerm,
storage
storage,
...presignedParams()
})
: HelpersService.loadTableRowCount({
workspace,
@@ -71,7 +85,14 @@
const workspace = workspaceId ?? $workspaceStore!
const csv = s3resource.endsWith('.csv')
if (appPath) {
const data = { workspace, path: appPath, fileKey: s3resource, storage, ...args }
const data = {
workspace,
path: appPath,
fileKey: s3resource,
storage,
...args,
...presignedParams()
}
return csv ? AppService.appLoadCsvPreview(data) : AppService.appLoadParquetPreview(data)
}
const data = { workspace, path: s3resource, storage, ...args }
@@ -230,7 +251,7 @@
{/if}
{#if !disable_download && !s3resource.endsWith('.csv')}
{@const csvApiPath = appPath
? `/w/${workspaceId}/apps_u/download_s3_parquet_file_as_csv/${appPath}?file_key=${encodeURIComponent(s3resource)}${storage ? `&storage=${storage}` : ''}`
? `/w/${workspaceId}/apps_u/download_s3_parquet_file_as_csv/${appPath}?file_key=${encodeURIComponent(s3resource)}${storage ? `&storage=${storage}` : ''}${presigned ? `&${presigned}` : ''}`
: `/w/${workspaceId}/job_helpers/download_s3_parquet_file_as_csv?file_key=${encodeURIComponent(s3resource)}${storage ? `&storage=${storage}` : ''}`}
{@const csvName = (s3resource.split('/').pop() ?? 'download') + '.csv'}
{#if shouldDownloadViaClient()}