mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 00:03:07 +00:00
* fix(apps): cover script/flow component outputs in deployed-app S3 provenance gate Deployed apps read S3 files on-behalf of the app author for logged-in viewers (#10048). A confused-deputy guard confines those reads to files the app "produced", but the recent-production check only matched inline `appscript`/ `preview` jobs nested under the app path. Files produced by the deployed script/flow components an app is wired to run (e.g. a SQL query persisted to S3) were therefore denied "File restricted" for every viewer, admins included. Expand the provenance check to also match completed `script`/`flow`/`flowscript`/ `flownode` jobs whose `runnable_path` is one of the app's declared triggerables, and accept the author identity via `permissioned_as = on_behalf_of` (not only `created_by = caller`) so files produced on-behalf of the author are covered. Reads outside the app's declared triggerables stay denied. Adds a regression test seeding a script-kind produced file that reproduces the "File restricted" denial before the fix and passes after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): key S3 provenance on on-behalf identity + cover flow steps (review) Addresses the CI review on the S3 provenance gate: - P1 (confused deputy): the recent-production check keyed on `created_by = caller`, so a viewer who can run a declared script/flow directly (outside the app, with un-pinned inputs) could craft a result naming an author-only key and read it back through the app as the author. Key provenance instead on the producing job's `permissioned_as` matching the on-behalf identity the download reads as (the author in author-mode); a viewer's direct run has `permissioned_as = viewer` and no longer clears the gate. Drops `created_by` from both the appscript/preview and script/flow branches, closing the same latent hole in the pre-existing inline-script branch. - P2 (dead flow-step branch): `flowscript`/`flownode` jobs have `runnable_path = <flow_path>/<step_id>`, which exact `= ANY(...)` never matched. Split script vs flow triggerable paths; flow kinds now match the flow's own job (bare path) and its step jobs via a `<flow_path>/%` prefix, bounded to declared flows. - P2 (test realism): the regression test now uses the production component-prefixed triggerable key format (`<id>:script/...`), exercises a flow-step-produced key, and asserts a viewer's own direct run of a declared script stays denied (the P1 case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): tie deployed-app S3 provenance to an app-origination marker (review) Second CI-review round flagged that `permissioned_as` still does not prove a job was app-launched: a runnable configured with its own `on_behalf_of` makes a direct `/jobs/run` resolve `permissioned_as` to that identity (the app author), so a viewer with run access could execute a declared runnable directly, craft an S3 result, and read it back through the app. The flow-path `LIKE fp || '/%'` match also let `_`/`%` in a declared path admit unrelated flows. Introduce a real app-origination marker instead of inferring provenance: - Add `JobTriggerKind::App`; `execute_component` stamps every app-launched job with `trigger_kind = 'app'` + `trigger = <app path>`. A direct `/jobs/run` cannot set this, so it is the authoritative signal that a file was produced *by the app*. - The provenance gate's recent-production check collapses to `trigger_kind = 'app' AND trigger = <this app path>` (+ the 3h window and result containment). This drops the forgeable `created_by`/`permissioned_as`/ `runnable_path`/kind logic entirely and removes the `LIKE` wildcard issue. - Provenance is scoped to THIS app's path, so another app's jobs (even same author) do not authorize this app's reads. Regression test rewritten to the marker model: an app-produced key clears for viewer and admin; a direct run whose `permissioned_as` resolves to the author stays denied (the forgery); another app's output stays denied. Adds `app` to the OpenAPI JobTriggerKind enum. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(apps): assert execute_component stamps trigger_kind='app' at runtime Adds an end-to-end test that runs a real script component through the app runtime (`apps_u/execute_component`) and asserts the enqueued job carries the app-origination marker `trigger_kind = 'app'` + `trigger = <app path>` (not the runnable path). The provenance-gate tests seed the marker directly; this proves the runtime actually produces the exact marker the gate depends on. execute_component commits the job row and returns its id, so the assertion reads the row directly — no worker needed to run the job. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(triggers): reject trigger_kind=app for suspended-job reassignment (review) `JobTriggerKind::App` (added for the app-origination S3 marker) became a valid value for the resume/cancel suspended-trigger routes, whose handler derives the table name `<kind>_trigger`. There is no `app_trigger` table, so both endpoints would fail with a missing-relation database error (500). Reject `App` in `get_suspended_trigger` alongside webhook/schedule so it returns a clean 400. Adds a regression test asserting the reassignment route returns 400 (not 500) for trigger_kind=app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): don't stamp app-origination marker on preview runs (review) The app-origination marker (trigger_kind='app') was stamped unconditionally, including preview mode. A preview lets a `jobs:run` caller supply arbitrary `raw_code` against ANY app path without that app's deployed policy (raw_code with no path/id skips all app authorization), so a preview returning `{"s3":"<author-only-key>"}` would forge the exact marker the S3 provenance gate trusts and read the victim app author's file. Gate the marker on `!is_preview`: only deployed, policy-checked executions are app-provenanced. Preview/editor S3 display does not rely on this marker (the editor routes reads through the force_viewer allowlist), so nothing legitimate regresses. Adds a regression test asserting a preview run's job is not stamped trigger_kind='app'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): editor-authorize preview marker + per-viewer S3 provenance isolation (review) Closes the codex P1 (preview forgery) without breaking editor preview downloads, and adds cross-viewer isolation to the provenance gate. - Preview marker now requires app write: `execute_component` stamps the app-origination marker on a preview only when the caller can EDIT that app (`require_is_writer`), instead of never stamping previews. An app editor already wields the app's author identity (they can deploy a component that reads the same file), so marking their own preview is no escalation and keeps preview-produced S3 results downloadable in the editor; a `jobs:run`-only caller who cannot edit the app still cannot forge the marker. Deployed runs are unchanged (always marked). - Per-viewer isolation: the provenance gate now also requires `j.created_by = <this caller>`. The security boundary stays the un-forgeable `trigger_kind='app'` marker; `created_by` is an additional filter ANDed under it, so it only narrows — a viewer can only download keys their OWN app runs produced, not another viewer's result. Restores the per-caller scoping #10048 had, now safe on top of the marker. Tests: preview marked iff caller can edit the app; cross-viewer isolation (another viewer's app-marked key denied, no admin bypass); direct-run and other-app keys still denied; deployed run still stamped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): require apps:write scope (not just writer ACL) to mark preview provenance (review) require_is_writer checks the user's underlying ACL but ignores token scopes, so a writer's token deliberately scoped to apps:run/apps:read/jobs:run but WITHOUT apps:write could still mark a preview and forge provenance — even though that token cannot deploy the app (update_app requires apps:write), breaking the "any marked caller can deploy equivalent code" rationale. Require BOTH apps:write:<path> scope (check_scopes) AND the writer ACL (require_is_writer) before stamping a preview's app-origination marker. Deployed runs unchanged. Adds a scope-restricted-writer token to the test (apps:run/read + jobs:run, no apps:write) and asserts its preview stays unmarked; retains the full-editor positive case and the non-editor negative case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): never app-provenance preview runs; read editor S3 as the caller (review) Simplifies the preview handling: a preview executes as the *caller* (Viewer mode), never as the author, so its results must be read back as the caller — never author-mode — and must never carry the app-origination marker. This removes the whole `require_is_writer` / `apps:write` / `can_preserve_on_behalf_of` reasoning (which was also unsound: a writer's token or session may not be able to deploy a component running as the app's on-behalf identity, so marking their preview could still escalate). - Backend: mark the app-origination marker for deployed runs only (`!is_preview`). - Frontend: `getS3File` (AppImage/AppPdf/AppDownload) now routes editor/preview reads through the viewer-scoped `job_helpers/download_s3_file` endpoint (reads as the caller), matching what DisplayResult/ParqetCsvTableRenderer already do; only a deployed app view uses the provenance-gated `apps_u` endpoint. This is the path that previously relied on marking previews, so nothing regresses. Test: a preview is never app-provenanced (owner's own preview and a non-editor's both stay unmarked). Cross-viewer isolation, deployed marking, and the reassignment guard are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(apps): app components run on-behalf of the app, not the referenced runnable (review) Root-causes codex's on-behalf-preview finding: `execute_component` was overriding the app's resolved on-behalf identity with the referenced script/flow's OWN `on_behalf_of` (its `on_behalf_of_email`). That is wrong in the app context — the app's execution mode should govern: - A Viewer-mode app could execute a component AS the referenced runnable's on_behalf identity (privilege confusion / escalation), instead of as the viewer. - A preview would run as that identity rather than as the caller, so its S3 output could not be read back as the caller — the download-identity mismatch codex flagged. Always use the app-resolved identity (author in author-mode, caller in viewer/preview); a referenced runnable's own `on_behalf_of` no longer leaks into app execution. Direct `/jobs/run` still honors a runnable's `on_behalf_of` (unchanged). With this, previews always run as the caller, so reading editor/preview S3 as the caller (viewer-scoped `job_helpers`) is unconditionally correct. - Test: the deployed-component e2e now seeds the script with a distinct on_behalf and asserts the component job's `permissioned_as` is the app identity, not the script's. - Also reword the getS3File `configuration` param comment to describe current state only (AGENTS.md comment rule). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(apps): surface 'app' trigger kind in Runs UI; condense provenance comments (review) Addresses codex review nits: - Add `app` to `jobTriggerKinds`, `triggerIconMap` (LayoutDashboard), and `triggerDisplayNamesMap` so app-component jobs (which now carry `trigger_kind = 'app'`) are filterable in Runs and render their trigger info. - Condense the app-origination marker, on-behalf-identity, and provenance-gate comments to state each invariant once in <=4 lines at its relevant site (AGENTS.md comment rule). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
169 lines
5.0 KiB
TypeScript
169 lines
5.0 KiB
TypeScript
import type { AppInput, EvalInputV2 } from '../inputType'
|
|
import type { App, RichConfigurations } from '../types'
|
|
import { collectOneOfFields } from './appUtilsCore'
|
|
function filenameExprToRegex(template: string) {
|
|
const filenameEscaped = template.replaceAll('${file.name}', '<file_name>') // replace filename with placeholder
|
|
const escapedTemplate = filenameEscaped
|
|
.slice(1, -1) // remove quotes
|
|
.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') // escape regex special characters
|
|
const regexPattern = escapedTemplate.replaceAll('<file_name>', '[^/]+') // replace filename placeholder with regex pattern
|
|
return `^${regexPattern}$`
|
|
}
|
|
|
|
function defaultIfEmptyString(str: string | undefined, dflt: string): string {
|
|
return str === undefined || str === null || str === '' ? dflt : str!
|
|
}
|
|
|
|
function staticToRegex(str: string) {
|
|
return `^${str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`
|
|
}
|
|
|
|
function checkIfExprIsString(input: string) {
|
|
return /^(['"`])[^'"`]*\1$/g.test(input)
|
|
}
|
|
|
|
function checkIfEvalIsStringWithFilename(input: EvalInputV2) {
|
|
if (input.connections.length > 0) {
|
|
return false
|
|
} else {
|
|
return checkIfExprIsString(input.expr.replaceAll('${file.name}', ''))
|
|
}
|
|
}
|
|
|
|
function removeResourcePrefix(resource: string) {
|
|
return resource.replace(/^\$res:/, '')
|
|
}
|
|
|
|
export function computeWorkspaceS3FileInputPolicy() {
|
|
return {
|
|
allow_workspace_resource: true,
|
|
allowed_resources: [],
|
|
allow_user_resources: false,
|
|
file_key_regex: ''
|
|
}
|
|
}
|
|
|
|
export function computeS3FileInputPolicy(s3Config: any, app: App) {
|
|
const resourceInput = s3Config?.resource as AppInput | undefined
|
|
const pathTemplateInput = s3Config?.pathTemplate as AppInput | undefined
|
|
|
|
const allow_workspace_resource =
|
|
!resourceInput || (resourceInput.type === 'static' && !resourceInput.value)
|
|
const allowed_resources: string[] = resourceInput
|
|
? resourceInput.type === 'static'
|
|
? resourceInput.value
|
|
? [removeResourcePrefix(resourceInput.value)]
|
|
: []
|
|
: (collectOneOfFields(
|
|
{
|
|
s3_resource: resourceInput
|
|
},
|
|
app
|
|
).s3_resource?.map((s) => removeResourcePrefix(s)) ?? [])
|
|
: []
|
|
|
|
const allow_user_resources =
|
|
(resourceInput?.type === 'evalv2' && resourceInput?.allowUserResources) ?? false
|
|
let file_key_regex = '^.*$'
|
|
if (pathTemplateInput) {
|
|
if (pathTemplateInput.type === 'static') {
|
|
file_key_regex = staticToRegex(pathTemplateInput.value)
|
|
} else if (
|
|
pathTemplateInput.type === 'evalv2' &&
|
|
checkIfEvalIsStringWithFilename(pathTemplateInput)
|
|
) {
|
|
file_key_regex = filenameExprToRegex(pathTemplateInput.expr)
|
|
}
|
|
}
|
|
|
|
return {
|
|
allow_workspace_resource,
|
|
allowed_resources,
|
|
allow_user_resources,
|
|
file_key_regex
|
|
}
|
|
}
|
|
|
|
export function isPartialS3Object(
|
|
input: unknown
|
|
): input is { s3: string; storage?: string; presigned?: string } {
|
|
return input != undefined && typeof input === 'object' && typeof input['s3'] === 'string'
|
|
}
|
|
|
|
export async function getS3File({
|
|
source,
|
|
storage,
|
|
presigned,
|
|
appPath,
|
|
username,
|
|
workspace,
|
|
token,
|
|
isEditor
|
|
}: {
|
|
source: string | undefined
|
|
storage?: string
|
|
presigned?: string
|
|
appPath: string
|
|
username: string | undefined
|
|
workspace: string
|
|
token: string | undefined
|
|
isEditor: boolean
|
|
// Optional; not read here. Editor reads go through the viewer-scoped endpoint
|
|
// and deployed reads through the app-scoped one, independent of the component
|
|
// configuration.
|
|
configuration?: RichConfigurations
|
|
}) {
|
|
if (!source) return ''
|
|
|
|
// Editor/preview runs execute as the *caller* (Viewer mode), so read their
|
|
// results back as the caller through the viewer-scoped `job_helpers` endpoint —
|
|
// never author-mode — consistent with DisplayResult/ParqetCsvTableRenderer. Only
|
|
// a deployed app view reads on-behalf of the author via the provenance-gated
|
|
// `apps_u` endpoint.
|
|
if (isEditor) {
|
|
const params = new URLSearchParams()
|
|
params.append('file_key', source)
|
|
if (storage) {
|
|
params.append('storage', storage)
|
|
}
|
|
if (token && token != '') {
|
|
params.append('token', token)
|
|
}
|
|
return `/api/w/${workspace}/job_helpers/download_s3_file?${params.toString()}${presigned ? `&${presigned}` : ''}`
|
|
}
|
|
|
|
const appPathOrUser = defaultIfEmptyString(appPath, `u/${username ?? 'unknown'}/newapp`)
|
|
const params = new URLSearchParams()
|
|
params.append('s3', source)
|
|
if (storage) {
|
|
params.append('storage', storage)
|
|
}
|
|
if (token && token != '') {
|
|
params.append('token', token)
|
|
}
|
|
|
|
return `/api/w/${workspace}/apps_u/download_s3_file/${appPathOrUser}?${params.toString()}${presigned ? `&${presigned}` : ''}`
|
|
}
|
|
|
|
export function computeS3FileViewerPolicy(config: RichConfigurations) {
|
|
if (config.source.type === 'uploadS3' && isPartialS3Object(config.source.value)) {
|
|
return {
|
|
s3_path: config.source.value.s3,
|
|
storage: config.source.value.storage
|
|
}
|
|
} else if (
|
|
config.source.type === 'static' &&
|
|
typeof config.source.value === 'string' &&
|
|
((config.sourceKind?.type === 'static' &&
|
|
config.sourceKind?.value === 's3 (workspace storage)') ||
|
|
config.source.value.startsWith('s3://'))
|
|
) {
|
|
return {
|
|
s3_path: config.source.value.replace('s3://', ''),
|
|
storage: undefined
|
|
}
|
|
} else {
|
|
return undefined
|
|
}
|
|
}
|