starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-06 16:02:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: a guest uses an anonymous app as itself; S3 uploads confined by app mode

Browse Source
This commit is contained in:
Ruben Fiszel
2026-09-02 15:21:17 +00:00
parent 7477976788
commit c8183e33e5
3 changed files with 103 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/app_guest_execution_mode.rs
+82 -14
View File
@@ -38,7 +38,9 @@ fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuil
async fn enable_guests(port: u16, ws: &str) -> anyhow::Result<()> {
authed(
client().post(format!("http://localhost:{port}/api/w/{ws}/workspaces/edit_guest_access")),
client().post(format!(
"http://localhost:{port}/api/w/{ws}/workspaces/edit_guest_access"
)),
ADMIN_TOKEN,
)
.json(&json!({ "guest_access_enabled": true }))
@@ -330,12 +332,16 @@ async fn a_self_declared_guest_scope_grants_nothing(db: Pool<Postgres>) -> anyho
/// gets past the triggerables lookup and reaches the guest gate. `sandbox` is what
/// makes the embed-token endpoint actually mint a token.
fn guest_app_with_runnable(path: &str, sandbox: bool) -> serde_json::Value {
app_with_runnable(path, "guest", sandbox)
}
fn app_with_runnable(path: &str, execution_mode: &str, sandbox: bool) -> serde_json::Value {
json!({
"path": path,
"summary": "Guest app",
"summary": "App",
"value": {},
"policy": {
"execution_mode": "guest",
"execution_mode": execution_mode,
"sandbox": sandbox,
"triggerables_v2": {
"script/u/test-user/noop": { "static_inputs": {}, "one_of_inputs": {} }
@@ -363,9 +369,7 @@ fn execute(port: u16, ws: &str, app: &str, token: &str) -> reqwest::RequestBuild
/// git-sync and execution once an admin has turned guests off — and it closes the
/// app to sessions already issued.
#[sqlx::test(fixtures("base"))]
async fn the_door_re_checks_the_workspace_switch(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
async fn the_door_re_checks_the_workspace_switch(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
@@ -386,7 +390,11 @@ async fn the_door_re_checks_the_workspace_switch(
let resp = authed(client().get(format!("{ws}/users/whoami")), GUEST_TOKEN)
.send()
.await?;
assert_eq!(resp.status(), 401, "a guest must not authenticate while guests are off");
assert_eq!(
resp.status(),
401,
"a guest must not authenticate while guests are off"
);
let resp = execute(port, "test-workspace", APP_PATH, GUEST_TOKEN)
.send()
.await?;
@@ -448,6 +456,59 @@ async fn guest_cannot_run_another_guest_app(db: Pool<Postgres>) -> anyhow::Resul
Ok(())
}
/// An anonymous app is open to anyone, a guest included, and the guest uses it as
/// itself: the component run and the result read that follows are one identity, so
/// the read's launched-by-me grant matches. Acting as nobody for the run and as the
/// guest for the read would start a job whose result the page can never fetch.
#[sqlx::test(fixtures("base"))]
async fn a_guest_uses_an_anonymous_app_as_itself(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");
enable_guests(port, "test-workspace").await?;
let resp = authed(client().post(format!("{ws}/scripts/create")), ADMIN_TOKEN)
.json(&json!({
"path": "u/test-user/noop",
"summary": "",
"description": "",
"content": "echo 42",
"language": "bash",
}))
.send()
.await?;
assert_eq!(resp.status(), 201, "{}", resp.text().await?);
let anon = "u/test-user/anon_app";
let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
.json(&app_with_runnable(anon, "anonymous", false))
.send()
.await?;
assert_eq!(resp.status(), 201, "{}", resp.text().await?);
insert_guest_token(&db, "test-workspace").await?; // scoped to APP_PATH, not `anon`
let resp = execute(port, "test-workspace", anon, GUEST_TOKEN)
.send()
.await?;
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
let job_id = resp.text().await?;
let resp = authed(
client().get(format!("{ws}/jobs_u/getupdate/{job_id}")),
GUEST_TOKEN,
)
.send()
.await?;
assert_eq!(
resp.status(),
200,
"the guest that started the run must be able to read it back: {}",
resp.text().await?
);
Ok(())
}
/// The embed token a guest mints for a sandboxed app is the one credential handed to
/// untrusted app JS. It must be a guest twice over — resolve like its minter (the
/// label) and be governed like its minter (the sentinel) — or every guest control
@@ -482,7 +543,12 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool<Postgres>) -> anyhow:
)
.send()
.await?;
assert_eq!(resp.status(), 200, "a guest must be able to mint: {}", resp.text().await?);
assert_eq!(
resp.status(),
200,
"a guest must be able to mint: {}",
resp.text().await?
);
let body: serde_json::Value = resp.json().await?;
let embed = body["token"]
.as_str()
@@ -492,11 +558,10 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool<Postgres>) -> anyhow:
// Its lifetime is capped at the session that minted it: the requested embed
// validity (12h) is longer than the guest session's (8h in this fixture), and the
// session's expiry is a guest's only revocation.
let parent_exp: chrono::DateTime<chrono::Utc> = sqlx::query_scalar(
"SELECT expiration FROM token WHERE token_prefix = 'GUEST_SECR'",
)
.fetch_one(&db)
.await?;
let parent_exp: chrono::DateTime<chrono::Utc> =
sqlx::query_scalar("SELECT expiration FROM token WHERE token_prefix = 'GUEST_SECR'")
.fetch_one(&db)
.await?;
let child_exp: chrono::DateTime<chrono::Utc> = body["expiration"]
.as_str()
.and_then(|e| e.parse().ok())
@@ -584,7 +649,10 @@ async fn a_guest_label_is_governed_without_the_sentinel(db: Pool<Postgres>) -> a
let ws = format!("http://localhost:{port}/api/w/test-workspace");
enable_guests(port, "test-workspace").await?;
let scopes: Vec<String> = guest_scopes().into_iter().filter(|s| s != "guest").collect();
let scopes: Vec<String> = guest_scopes()
.into_iter()
.filter(|s| s != "guest")
.collect();
sqlx::query(
"INSERT INTO token (token_hash, token_prefix, token, email, label, scopes, workspace_id, expiration)
VALUES (encode(sha256($1::bytea), 'hex'), 'NOSENTINE_', $2, 'guest@example.com',
backend/windmill-api/src/apps.rs
+15 -14
View File
@@ -365,10 +365,10 @@ pub fn authorize_non_member_viewer(
Ok(false)
}
/// The caller an app-scoped operation proceeds with, once the app's mode is known. A
/// guest session names one app: on an anonymous app a mismatch means "act as anyone",
/// on any other it is the refusal the page answers with a fresh sign-in. Non-guest
/// scoped callers are confined at the top of each handler instead, before any read.
/// Confines a guest to its app once the app's mode is known; a no-op for every other
/// caller. An anonymous app is open to anyone, so the guest stays the caller there,
/// as itself: the run and the reads that follow it (job results, S3 provenance) must
/// carry one identity. Anywhere else a mismatch is refused.
fn guest_caller_for_mode(
opt_authed: Option<ApiAuthed>,
mode: ExecutionMode,
@@ -377,16 +377,14 @@ fn guest_caller_for_mode(
let Some(authed) = opt_authed.as_ref() else {
return Ok(None);
};
if !windmill_api_auth::scopes::has_guest_sentinel(authed.scopes.as_deref()) {
if !windmill_api_auth::scopes::has_guest_sentinel(authed.scopes.as_deref())
|| matches!(mode, ExecutionMode::Anonymous)
{
return Ok(opt_authed);
}
match check_scopes(authed, || format!("apps:run:{app_path}"))
.or_else(|_| check_scopes(authed, || format!("apps:read:{app_path}")))
{
Ok(()) => Ok(opt_authed),
Err(_) if matches!(mode, ExecutionMode::Anonymous) => Ok(None),
Err(e) => Err(e),
}
check_scopes(authed, || format!("apps:run:{app_path}"))
.or_else(|_| check_scopes(authed, || format!("apps:read:{app_path}")))?;
Ok(opt_authed)
}
/// [`authorize_non_member_viewer`] plus the member read-access probe, for the
@@ -4461,8 +4459,7 @@ async fn upload_s3_file_from_app(
request: axum::extract::Request,
) -> JsonResult<AppUploadFileResponse> {
// Same path confinement as `execute_component`: without it a token scoped to app A
// could drive app B's upload policy. A guest's waits for the app's mode, inside
// `get_on_behalf_authed_from_app`.
// could drive app B's upload policy. A guest's waits for the app's mode, below.
if let Some(authed) = opt_authed
.as_ref()
.filter(|a| !windmill_api_auth::scopes::has_guest_sentinel(a.scopes.as_deref()))
@@ -4519,6 +4516,10 @@ async fn upload_s3_file_from_app(
.map(|p| serde_json::from_value::<Policy>(p).map_err(to_anyhow))
.transpose()?
};
let opt_authed = match policy.as_ref() {
Some(policy) => guest_caller_for_mode(opt_authed, policy.execution_mode(), path.to_path())?,
None => opt_authed,
};
let user_db = UserDB::new(db.clone());
frontend/src/lib/components/apps/editor/PublicAppFrame.svelte
+6 -5
View File
@@ -209,10 +209,9 @@
type FrameStatus = 'loading' | 'ready' | 'noPermission' | 'notExists' | 'sdkPrompt'
let status = $state<FrameStatus>('loading')
/** Whether the visitor holds an account session, probed whenever the app denies
* them: `/api/users/email` answers for an account and never for a guest (pinned
* to its workspace) or for nobody. An account this app still refuses is not
* something signing in again can fix — an identity with an account is never given
* a guest session — so the card gives way to an explanation. */
* them. An account this app still refuses is not something signing in again can
* fix — an identity with an account is never given a guest session — so the card
* gives way to an explanation. */
let accountSession = $state<'unknown' | 'none' | 'held'>('unknown')
let deniedStatus: number | undefined = $state(undefined)
/** The sign-in card belongs on a 401, and on a 403 unless discovery has settled
@@ -227,6 +226,8 @@
let signInDidNotHelp = $derived(offerSignIn && accountSession === 'held')
$effect(() => {
if (offerSignIn && accountSession === 'unknown') {
// Workspace-less, so it answers for an account and never for a guest
// (pinned to its workspace) or for nobody.
UserService.getCurrentEmail()
.then(() => (accountSession = 'held'))
.catch(() => (accountSession = 'none'))
@@ -603,7 +604,7 @@
You are signed in, but this app is not open to you
</div>
<div class="text-center mt-8 text-sm text-primary">
It is open to members of its workspace{guestAppPath
It is open to the people it was shared with{guestAppPath
? ', and to guests who have no Windmill account'
: ''}. Ask the person who shared it to give your account access.
</div>