Skip to content

Conversation

@nolanpro
Copy link
Contributor

@nolanpro nolanpro commented Jan 16, 2026

ci:k8s-branch:2026-3-php84
ci:package-auth:task/FOUR-28803
ci:package-email-start-event:task/FOUR-28803
ci:package-collections:task/FOUR-28803

.........


Note

Upgrades the framework/runtime and aligns code with new APIs and behaviors.

  • Upgrade runtime/deps: Bumps to php@8.4, laravel/framework@^12, laravel/passport@^13, and other libraries in composer.json
  • Auth clients: Replaces Passport controller inheritance with a custom ClientController using ClientRepository (findForUser, explicit client creation per grant type), returns 404 on missing, and exposes secret on create
  • Session/auth: Simplifies AuthenticateSession to act only with SessionGuard and defer to parent; removes middlewarePriority from Http\Kernel; sets Passport::$clientUuids = false
  • DB/schema utilities: Updates CreateDataLakeViews to pass the database name to Schema::getTables/getViews
  • Tooling: Adds --ssl=false to mysqldump in CreateTestDBs; reduces BuildScriptExecutor retries to 1
  • Chore: Adds .envrc to .gitignore

Written by Cursor Bugbot for commit 578ebc7. This will update automatically on new commits. Configure here.

$views = array_map(function ($item) {
return $item['name'];
}, Schema::getViews());
}, Schema::getViews($database));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getViews returns incompatible data structure breaking view logic

High Severity

The getViews() method now returns a numerically-indexed array of view name strings, but consumers expect an associative array keyed by view name with objects having a getSql() method. In shouldCreate(), the check isset($views[$viewName]) will always fail since the array uses numeric keys, causing views to always be recreated unnecessarily. In the up() method's foreach loop, $viewName becomes numeric indices (0, 1, 2...) instead of actual view names, breaking the dropped table detection logic entirely.

Additional Locations (2)

Fix in Cursor Fix in Web

true, // confidential
$request->user()
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store method silently ignores multiple selected client types

Medium Severity

The store() method uses an if-elseif-else structure that only creates one client type, silently ignoring additional types when multiple are selected. The validation allows multiple types (array|min:1), and the update() method correctly handles both personal_access_client and password_client flags independently. However, store() only honors the first matching type—if both personal access and password are requested, only the personal access client is created. This creates an inconsistency where capabilities available via update are not available during creation.

Fix in Cursor Fix in Web

@nolanpro nolanpro changed the title Upgrade to Laravel 12 and PHP 8.5 Upgrade to Laravel 12 and PHP 8.4 Jan 23, 2026
$tables = array_map(function ($item) {
return $item['name'];
}, Schema::getTables());
}, Schema::getTables($database));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Schema methods don't accept database parameter

High Severity

Schema::getTables() and Schema::getViews() methods in Laravel 12 don't accept a database name parameter. The code passes $database to these methods, but they take no arguments. This would cause an ArgumentCountError at runtime when the processmaker:create-data-lake-views artisan command is executed.

Additional Locations (1)

Fix in Cursor Fix in Web

$request->name,
null, // provider
false // confidential
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OAuth clients not associated with user when created

High Severity

When creating personal access or password grant clients via store(), the new code uses createPersonalAccessGrantClient() and createPasswordGrantClient() which don't associate the client with the authenticated user. The old code passed $request->user()->getKey() to link all client types to the user. Since show(), update(), and destroy() all use findForUser($clientId, $request->user()) to retrieve clients, users can no longer access, modify, or delete personal access and password grant clients they create through this API.

Additional Locations (2)

Fix in Cursor Fix in Web

$request->name,
null, // provider
false // confidential
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Password grant client created as non-confidential breaks database

High Severity

The createPasswordGrantClient() call passes false for the confidential parameter, but the oauth_clients database table has a NOT NULL constraint on the secret column. Non-confidential clients don't receive a secret, which will cause a database constraint violation. The UserSeeder explicitly uses true with a comment explaining "must be true as database requires secret to be NOT NULL", but the controller uses the opposite value.

Fix in Cursor Fix in Web

@processmaker-sonarqube
Copy link

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

$dbConnectionArgs = '-h ' . env('DB_HOSTNAME') . ' -P ' . env('DB_PORT') . ' -u ' . env('DB_USERNAME') . " -p'" . env('DB_PASSWORD') . "'";
$file = tempnam(sys_get_temp_dir(), 'dump');
$cmd = "mysqldump $dbConnectionArgs " . env('DB_DATABASE') . " > $file";
$cmd = "mysqldump --ssl=false $dbConnectionArgs " . env('DB_DATABASE') . " > $file";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symfony Process constructor requires array not string

High Severity

The Process constructor is called with a string command, but Symfony 7.x (included with Laravel 12) requires an array. Since the commands use shell features like output redirection (>, <), they need Process::fromShellCommandline($cmd) instead of new Process($cmd). This will throw a TypeError at runtime when the processmaker:create-test-dbs command is executed.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants