Skip to content

Refactor environment loading and handling for FrankenPHP + add more s…#397

Merged
tudor-timcu merged 8 commits intomainfrom
load_environment_refactoring
Feb 20, 2026
Merged

Refactor environment loading and handling for FrankenPHP + add more s…#397
tudor-timcu merged 8 commits intomainfrom
load_environment_refactoring

Conversation

@ioaniftimesei
Copy link
Contributor

@ioaniftimesei ioaniftimesei commented Feb 19, 2026

…afe-guards for AIKIDO_DISABLE

Summary by Aikido

Security Issues: 0 🔍 Quality Issues: 6 Resolved Issues: 0

⚡ Enhancements

  • Added system and frankenphp-specific environment fetchers with logging
  • Added runtime guards to skip processing when AIKIDO_DISABLE was set

🔧 Refactors

  • Reworked environment loading into prioritized getters and helper functions
  • Changed initialization to call LoadSystemEnvironment during MINIT
  • Bumped library version constants to 1.5.2 across codebase
  • Added missing include for vector and minor include reordering

More info

@@ -185,12 +190,16 @@ bool GetBoolFromString(const std::string& env, bool default_value) {
return default_value;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's rename this function to GetEnvBoolWithAllGetters or smth like this so it's more explicit what it does. Can remove the comment from the Includes.h after that.

std::string sapiName = sapi_module.name;

if (sapiName == "frankenphp") {
if (GetEnvBoolWithAllGetters("FRANKENPHP_WORKER", false)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling GetEnvBoolWithAllGetters("FRANKENPHP_WORKER", ...) here can dereference AIKIDO_GLOBAL(server) via GetFrankenEnvVariable before server is initialized; guard AIKIDO_GLOBAL(server) or delay this check.

Details

✨ AI Reasoning
​The code attempts to detect a FrankenPHP warm-up request by calling GetEnvBoolWithAllGetters("FRANKENPHP_WORKER", false) early in RequestProcessor::RequestInit. GetEnvBoolWithAllGetters uses the complete set of getters including GetFrankenEnvVariable, which calls AIKIDO_GLOBAL(server).GetVar. Because the FRANKENPHP_WORKER check was moved earlier in RequestInit (before environment/server-related initialization and before LoadEnvironment/LoadLaravelEnvFile), AIKIDO_GLOBAL(server) may not be initialized yet, leading to dereferencing an uninitialized or null server object and causing a segmentation fault. This regression was introduced by moving the FRANKENPHP_WORKER check to earlier lines and by using the "all getters" helper at that point.

🔧 How do I fix it?
Add null checks before dereferencing pointers, validate array bounds before access, avoid using pointers after free/delete, don't write to string literals, and prefer smart pointers in modern C++.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@tudor-timcu tudor-timcu merged commit 1db4fa8 into main Feb 20, 2026
42 checks passed
@tudor-timcu tudor-timcu deleted the load_environment_refactoring branch February 20, 2026 13:25
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