GeezerVison

FlowGenie updates wipe secrets

High Severity

update replaces the entire config JSON with normalize(), which merges the payload onto hardcoded defaults. Partial updates therefore reset tools, MCP servers, and provider_api_key. get/detail also put [redacted] in those secret fields, so a get-then-update writes the placeholder over the real key and OAuth tokens.

 

^{Reviewed by Cursor Bugbot for commit 9214abe. Configure here.}

FlowGenie list omits config

Medium Severity

list paginates FlowGenies without selecting config, but summary() reads model, provider, output_type, and has_instructions from that attribute. Eloquent leaves omitted columns null, so every list row reports empty model/provider and has_instructions: false.

 

^{Reviewed by Cursor Bugbot for commit 9214abe. Configure here.}

Archived processes cannot be listed

Medium Severity

list always applies notArchived() and still accepts status=ARCHIVED. The two filters contradict, so a request for archived processes returns an empty page even though update can set that status.

 

^{Reviewed by Cursor Bugbot for commit 9214abe. Configure here.}

Collection create skips type validation

Medium Severity

After validating with Collection::rules(), create assigns type from the raw input. Update restricts type to NORMAL or RAG; create does not, so arbitrary values can be stored and later break listing or RAG behavior.

 

^{Reviewed by Cursor Bugbot for commit 9214abe. Configure here.}

🔒 Agentic Security Review

Severity: HIGH

Description: get-flow-genie returns the stored FlowGenie config after a fixed denylist. redactConfig masks only top-level provider_api_key and, under mcp_servers[].config.http_config.authentication, the names access_token, client_secret, cached_access_token, and cached_refresh_token (also those same names inside oauth_pkce and oauth_client_credentials). It does not walk the rest of the authentication object, http_config siblings, or header maps. FlowGenie Studio persists two additional MCP auth methods, Access Token/API Key and Custom Headers, whose secret values are not those four names. find() loads any FlowGenie by id for any caller with view-flow_genies and returns that config from get-flow-genie (create and update use the same detail() path).

Impact: A designer with view-flow_genies and an API token can read another FlowGenie's MCP credentials that the new write-only contract says are redacted. Those values are returned in the tool payload, so they also enter the MCP client and model transcript. Top-level provider_api_key and the four OAuth token names are masked; Access Token/API Key material and custom header secrets are not. This is a new read into a destination the PR tried to close. Nested subagent provider_api_key is not part of this finding: subagents are references to other genies plus a handoff description, not an embedded copy of the child config.

Remediation: Stop returning credential material from FlowGenie reads. Omit mcp_servers[].config.http_config.authentication (and any header map) the way DataConnectorCatalog::detail omits credentials, or recursively redact secret-bearing values at any depth: provider_api_key, token, api_key, password, secret, refresh_token, code_verifier, and header values such as Authorization, including Access Token/API Key and Custom Headers. Do not rely on a four-name denylist on one authentication object. Add a test that stores an API key and a custom Authorization header, calls get-flow-genie as a different user who has only view-flow_genies, and asserts those values are absent.

Plugin names allow path traversal

High Severity

Uninstall, toggle, reinstall, and git install join storage/plugins with the raw plugin name or URL basename. A name like .. resolves to the tenant storage directory, so those operations can delete or overwrite the whole storage tree. The API {name} route and extractRepoName do not restrict the value to a single safe path segment.

 

^{Reviewed by Cursor Bugbot for commit 9420bd0. Configure here.}

Failed overwrite drops existing plugin

High Severity

Zip and local installs call markNewInstallForRollback while the old plugin directory still exists, so rollback is skipped, then they delete that directory. If copy, validation, or composer install fails afterward, revertFailedInstall does nothing and the previously working plugin is gone.

 

^{Reviewed by Cursor Bugbot for commit 9420bd0. Configure here.}

Zip cleanup uses unset name

Medium Severity

The installFromZip finally block always passes $repoName into deleteDirectory, but that variable is assigned only after extractTo, close, and scandir. A failure in those steps leaves $repoName undefined, so Laravel can turn the warning into an exception that hides the original zip error and skip temp cleanup.

 

^{Reviewed by Cursor Bugbot for commit 9420bd0. Configure here.}

Per-page clamp uses floor plus one

Medium Severity

changePerPage sets the last page with Math.floor(total / value) + 1 instead of Math.ceil. When the total is an exact multiple of the new page size, the list stays on a page past the last one, so the table is empty while the footer still implies extra rows.

 

^{Triggered by learned rule: Pagination last-page is ceil; reset modal page state}

^{Reviewed by Cursor Bugbot for commit 9420bd0. Configure here.}

🔒 Agentic Security Review

Severity: MEDIUM

Description: POST /api/1.0/plugins/install copies the caller-supplied git_token onto public InstallPlugin::$gitToken. Laravel serializes that property into the queue payload, so the PAT stays in Redis, Horizon job history, and failed_jobs after the clone. The same change only redacts the token from log and exception text.

Impact: A manage-plugins user can submit a GitHub PAT or other git password that is then retained outside the clone. Default queueing is Redis (config/queue.php), Horizon keeps the full payload for completed jobs (60 minutes) and failed jobs (7 days), and failed jobs are written to failed_jobs with the raw serialized command. Horizon's viewHorizon gate and the tenant queue detail API are administrator-only, but Redis, database backups, and those admin views are still a different authority than the installer, and the token remains reusable after the install finishes or fails. The PR already treats this value as a secret: PluginManager strips it from the git remote and redactHttpUserInfoFromString is applied before PluginLog and timeout errors.

Remediation: Do not put gitUsername or gitToken on the queued job. Store the token in a short-lived server-side secret (cache/encrypted row) addressed by an opaque id, delete it as soon as the remote URL is set, and keep InstallPlugin free of the raw credential. If the payload must carry it, implement ShouldBeEncrypted and still remove the secret from the job before it can be written to failed_jobs.

In-process scripts ignore timeout

High Severity

Default modules execution computes a timeout and stores it on ScriptExecutionContext, but ScriptRuntime::run never reads timeout and does not cap memory either. A sleep, infinite loop, or heavy allocation can block a Horizon worker indefinitely or OOM it. Only bare mode actually enforces Process::timeout() and memory_limit.

 

^{Reviewed by Cursor Bugbot for commit 0c20653. Configure here.}

Included scripts can kill workers

Medium Severity

Default modules mode includes user PHP in the Laravel worker. exit/die (common in PHP scripts) terminates the worker immediately, skipping the finally that restores putenv state and deletes the generated API token, and it can leave the BPMN token stuck.

 

^{Reviewed by Cursor Bugbot for commit 0c20653. Configure here.}

Preview bypasses in-process allowlist

High Severity

When Core Service Task is enabled, run_in_process=true on script preview runs caller-supplied PHP in the queue worker without allow_in_process. That skips Docker isolation and runs as run_as_user_id with an API token, so any user who can preview a PHP script can execute arbitrary code in the ProcessMaker process.

 

^{Reviewed by Cursor Bugbot for commit 0c20653. Configure here.}

Subprocess inherits worker secrets

Medium Severity

buildEnvironment copies the entire parent $_ENV and $_SERVER into the bare PHP subprocess. Unlike the Docker runner, that includes APP_KEY, database credentials, and other worker secrets, so in-process scripts receive far more privileged environment than isolated executors.

 

^{Reviewed by Cursor Bugbot for commit 0c20653. Configure here.}

🔒 Agentic Security Review

Severity: HIGH

Description: POST /api/1.0/scripts/{script}/preview host-executes the request body's PHP when the caller sends run_in_process=true, or whenever the saved script has allow_in_process, without requiring the executed bytes to be that persisted trusted script. The route is only can:view-scripts. ScriptRunner and CoreServiceTask both refuse in-process execution unless the script record has allow_in_process, and config/core-service-task.php documents that limit, but shouldPreviewInProcess returns true on the client boolean first. TestScript then overwrites the in-memory script with the request code and, in the default modules mode, ScriptRuntime::run include()s it in the queue worker after putenv of every EnvironmentVariable and a run-as user's API_TOKEN.

Impact: A user granted view-scripts (included on the default Process Designer role; requesters do not have it) can run arbitrary PHP inside the Laravel worker once CORE_SERVICE_TASK_ENABLED is on. That is the worker process, not the Docker or script-microservice sandbox preview used before: the included code can read the worker environment and database, the injected environment-variable secrets, and an API token for the script's run-as user. The same caller cannot set allow_in_process unless they also have edit-scripts, so this skips the host-execution trust gate the rest of the change enforces. The feature flag defaults off; the hole is the enabled configuration this PR adds.

Remediation: Do not honor a client run_in_process flag. Host-execute preview only when the saved script has allow_in_process and the executed bytes are that persisted script (ignore or reject a differing code body). Keep the ScriptRunner and CoreServiceTask allow_in_process checks as the only in-process gate.

🔒 Agentic Security Review

Severity: MEDIUM

Description: Modules-mode script execution stores a capped timeout on ScriptExecutionContext and then never enforces it. ScriptRuntime::run() include()s the PHP script in the current worker with no set_time_limit, alarm, or subprocess timeout. The bare runner (InProcessPhpRunner) applies the same cap via Process::timeout(). CLI workers are configured with max_execution_time=0, and TestScript, ExecuteScript, RunScriptTask, and RunNayraServiceTask set no job $timeout or retryUntil, so the only remaining kill is the Horizon bpmn supervisor timeout (default 3600s) via pcntl_alarm. That backstop is not the configured max_timeout (default cap 300s), and because the script runs in the worker it can clear it with pcntl_alarm(0) or pcntl_signal(SIGALRM, SIG_IGN) and then loop. Default bpmn maxProcesses is 1 and redis retry_after is 86400, so one run holds the shared BPMN worker for the full supervisor timeout (or until an operator kills it). SIGKILL also skips the finally block, so the script-runner Passport token created for the run is not deleted.

Impact: A Process Designer (view-scripts / edit-scripts), or a preview caller who can set run_in_process, can stall the shared bpmn queue past core-service-task.max_timeout once CORE_SERVICE_TASK_ENABLED is on. Execution mode defaults to modules. With the shipped Horizon pool of one bpmn worker, every other process, timer, and script task waits. A plain tight loop is reaped only at the 3600s supervisor timeout; disarming SIGALRM in the same process removes that kill. The script-runner access token then stays valid because GenerateAccessToken::delete() never runs. This is a fail-open of the execution limit the new in-process path claims to apply, on a hot shared worker, not a self-scoped delay.

Remediation: Enforce the already-capped context timeout around modules execution and fail the job with the same ScriptTimeoutException the bare runner raises. An in-process set_time_limit or pcntl_alarm is not enough: the included script can call set_time_limit(0) or pcntl_alarm(0). Use a watcher the script cannot clear (subprocess Process::timeout, as in InProcessPhpRunner, or a parent that is not the included script) and delete the script-runner Passport token even when the run is aborted.

🔒 Agentic Security Review

Severity: HIGH

Description: Bare in-process PHP execution forwards the queue worker's environment, including application secrets, into the script subprocess. buildEnvironment() starts from array_merge($_ENV, $_SERVER) and Process::env() passes that array to Symfony Process. Symfony Process 8.1 then overlays the parent environment (getDefaultEnv()) and does not drop keys that were merely omitted, so APP_KEY, DB_PASSWORD, and the rest of the worker environment remain readable with getenv() inside script.php. The script return value is JSON-encoded to stdout and returned to the caller.

Impact: A script author is a lower-trust actor than the worker. Docker execution only injects EnvironmentVariable rows, HOST_URL, smart-extract settings, and a run-as API token. Bare mode is the supported subprocess path (CORE_SERVICE_TASK_EXECUTION other than modules) whose purpose is a PHP process without Laravel, but it still exposes the worker credential set. The script can return those values in preview output or process data, or send them off-box. APP_KEY and database credentials are full application credentials, not script-scoped configuration. This path is off by default: core-service-task is disabled and execution defaults to modules, which does not call InProcessPhpRunner. It is reachable once an operator sets CORE_SERVICE_TASK_ENABLED=true and CORE_SERVICE_TASK_EXECUTION=bare. In that configuration an authenticated user with view-scripts can POST scripts/{script}/preview with run_in_process=true and arbitrary PHP (allow_in_process is not required). The same runner is also used for allow_in_process PHP script tasks and core service tasks.

Remediation: Build the subprocess environment from the same allowlist as Base::getEnvironmentVariables (EnvironmentVariable rows, HOST_URL/APP_URL, smart-extract settings, and the run-as API token). Do not merge $_ENV or $_SERVER into that array. Because Symfony Process 8 still merges getDefaultEnv(), set every inherited name that is not on the allowlist to false before proc_open so those variables are dropped. Add a test that getenv('APP_KEY') and getenv('DB_PASSWORD') are empty in the child while API_TOKEN and an EnvironmentVariable row are present.

🔒 Agentic Security Review

Severity: MEDIUM

Description: Tenant plugin asset publish dereferences symlinks under the plugin public/ tree and writes the target bytes into the tenant document root as a normal file. git clone (no core.symlinks=false) leaves those links in storage/tenant_{id}/plugins/{repo}/public. publishPlugin() accepts the tree when is_dir() is true, which follows a symlink, and copyPublicDirectory() calls File::copyDirectory() with no is_link skip and no realpath containment. Laravel's copier uses is_dir()/SplFileInfo::isDir() and PHP copy(), both of which follow links, so a file symlink is materialized and a directory symlink is recursed into. A plugin repository can therefore point public/leak at a stable relative path such as ../../../../../.env (from storage/tenant_{id}/plugins/{repo}/public up to the app root) and the file contents are written under public/storage/tenant_{id}/vendor/processmaker/packages/{package}/, which is the static URL prefix /storage/tenant_{id}/vendor/processmaker/packages/{package}/. That publish runs from finishPluginSetup after clone, including when composer.json has no scripts and no extra.laravel.providers, so the copy itself is enough to place the bytes on disk.

Impact: A plugin repository author is a lower-trust actor than the host filesystem. An operator with manage-plugins who installs that https git URL (multitenancy on, tenant_plugin_assets_enabled default true, queue job tenant-aware) causes host files readable by the PHP user, including the application .env and other credential files, to be stored inside the web root. The resulting object is an ordinary file on the tenant public disk (public/storage/tenant_{id} is the tenants:storage-link path into storage/app/public, and a missing link still creates a directory under public/), so it is fetched as a static asset with no application auth. That crosses the plugin public-directory boundary the new publisher is responsible for. Zip extraction was not verified to recreate symlink entries; the confirmed path is git clone into the plugin directory followed by this publish.

Remediation: While publishing plugin public assets, skip symlink entries (is_link) and do not recurse through directory symlinks. Before each write, require realpath of the source file to remain under realpath of the plugin public directory, and require the destination to remain under the tenant package directory. Apply the same containment in PluginManager::copyDirectory so a local or extracted tree cannot materialize out-of-tree targets before publish.

🔒 Agentic Security Review

Severity: MEDIUM

Description: The new Laya extras manifest depends on PyPI project laya-evals>=0.1.0. That project is owned by PyPI user gjxv (github.com/Gjusev/laya-evals), not Convai Innovations, and its own description says it is not the laya-evals CLI shipped with upstream laya. The quality harness only imports laya.Router. README.md and run_laya_eval.sh still tell an operator to pip install -r requirements.txt, so that install trusts every later gjxv release. Published laya-evals 0.1.0 has no console script, so this release does not itself overwrite upstream laya's laya-evals entry point; the open range lets a later release add that script or run code from its build backend.

Impact: An operator or agent who follows the new optional Laya install instructions installs and can execute code from an unrelated publisher in the same environment as laya. That is a new provenance trust of a third-party PyPI project, with an unbounded upper version, on a name that collides with upstream laya's documented eval CLI. Repo CI does not install this file today; the blast radius is every environment that opts into the documented requirements file, including a later in-range release chosen by that publisher.

Remediation: Remove laya-evals from requirements.txt. The harness only needs the pinned laya distribution, which already documents its own laya-evals CLI. If a separate extra is intentional, depend on a hash-pinned artifact from the same publisher as laya, not the colliding PyPI name on an open range.