app/controllers/general.php (1)

359-365: Naming settled; “client” namespace aligns with earlier discussion.

• Using x-appwrite-client-ip is consistent with the prior decision to use “client” for internet clients and to avoid implying authenticated users. No further action needed.

src/Appwrite/Platform/Workers/Functions.php (1)

402-412: Don’t clobber upstream client IP; optionally adopt header executionId when param is missing

• If controllers already populated x-appwrite-client-ip, this unconditionally overwrites it with an empty string, losing information. Set it only when it’s absent.
• For event/schedule paths where $executionId may be null/empty, consider adopting an upstream x-appwrite-execution-id header if present to preserve single-source execution ID across the pipeline.
• Header names LGTM and align with prior discussion.

Apply:

-        $headers['x-appwrite-execution-id'] = $executionId ?? '';
+        // Prefer provided executionId; if missing, adopt one from headers (e.g., controllers)
+        if (empty($executionId) && !empty($headers['x-appwrite-execution-id'])) {
+            $executionId = $headers['x-appwrite-execution-id'];
+        }
+        $headers['x-appwrite-execution-id'] = $executionId ?? '';

         $headers['x-appwrite-key'] = API_KEY_DYNAMIC . '_' . $apiKey;
         $headers['x-appwrite-trigger'] = $trigger;
         $headers['x-appwrite-event'] = $event ?? '';
         $headers['x-appwrite-user-id'] = $user->getId() ?? '';
         $headers['x-appwrite-user-jwt'] = $jwt ?? '';
         $headers['x-appwrite-country-code'] = '';
         $headers['x-appwrite-continent-code'] = '';
         $headers['x-appwrite-continent-eu'] = 'false';
-        $headers['x-appwrite-client-ip'] = '';
+        // Do not overwrite IP if controller already set it; default to empty string otherwise
+        $headers['x-appwrite-client-ip'] = $headers['x-appwrite-client-ip'] ?? ($headers['x-forwarded-for'] ?? '');

app/controllers/general.php (1)

377-388: Prefer robust client-IP resolution for geolocation.

• You currently geolocate by x-real-ip only. In environments where that header isn’t present, you’ll miss geodata while still sending x-appwrite-client-ip from Request::getIP().
• Optional: fall back to $request->getIP() (or first IP from X-Forwarded-For) when x-real-ip is empty, so geodata remains consistent with the client IP you expose to functions.

Example (outside the changed range, for illustration only):

$ip = $headers['x-real-ip'] ?? '';
if (empty($ip)) {
    $xff = $headers['x-forwarded-for'] ?? '';
    $ip = $xff ? \trim(\explode(',', $xff)[0]) : $request->getIP();
}

src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php (1)

224-226: Add test coverage for header override protection.

• We already test non-override for several headers, but not for x-appwrite-execution-id. Consider extending tests to assert that user-supplied x-appwrite-execution-id is ignored/overridden.

Proposed addition (tests/e2e/Services/Functions/FunctionsCustomClientTest.php, method testNonOverrideOfHeaders):

         $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([
             'content-type' => 'application/json',
             'x-appwrite-project' => $this->getProject()['$id'],
         ], $this->getHeaders()), [
             'x-appwrite-event' => "OVERRIDDEN",
             'x-appwrite-trigger' => "OVERRIDDEN",
             'x-appwrite-user-id' => "OVERRIDDEN",
             'x-appwrite-user-jwt' => "OVERRIDDEN",
+            'x-appwrite-execution-id' => "OVERRIDDEN",
+            'x-appwrite-client-ip' => "255.255.255.255",
         ]);
 
         $output = json_decode($execution['body']['responseBody'], true);
         $this->assertNotEquals('OVERRIDDEN', $output['APPWRITE_FUNCTION_JWT']);
         $this->assertNotEquals('OVERRIDDEN', $output['APPWRITE_FUNCTION_EVENT']);
         $this->assertNotEquals('OVERRIDDEN', $output['APPWRITE_FUNCTION_TRIGGER']);
         $this->assertNotEquals('OVERRIDDEN', $output['APPWRITE_FUNCTION_USER_ID']);
+        $this->assertNotEquals('OVERRIDDEN', $output['APPWRITE_FUNCTION_EXECUTION_ID']);
+        $this->assertNotEquals('255.255.255.255', $output['APPWRITE_FUNCTION_CLIENT_IP']);

tests/e2e/Services/Functions/FunctionsCustomClientTest.php (1)

162-166: Great assertions for execution ID and client IP.

• Verifying APPWRITE_FUNCTION_EXECUTION_ID matches the execution document ID and that APPWRITE_FUNCTION_CLIENT_IP is non-empty gives strong end-to-end coverage.

Optional: Also assert the ID format (e.g., not empty and matches your ID charset/length) to catch regressions in ID generation.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

app/controllers/general.php (1)

359-361: Single-source executionId propagation looks correct.

• Generating the executionId once and reusing it for both the header and the execution document mitigates correlation bugs and simplifies tracing. Good call.

src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php (1)

224-226: Good: single executionId + client IP propagated into function context.

• Creating executionId once and overriding any inbound x-appwrite-execution-id prevents spoofing. Setting x-appwrite-client-ip from Request::getIP() is appropriate.

Also applies to: 233-234

tests/resources/functions/basic/index.js (1)

44-46: Tests helper now surfaces execution and client IP — LGTM.

• Pulling x-appwrite-execution-id and x-appwrite-client-ip from headers and echoing them in the JSON response is aligned with the new behavior.

tests/e2e/Services/Functions/FunctionsCustomServerTest.php (1)

1337-1340: Server-side E2E parity — looks good.

• Mirroring the client test here reduces the chance of environment-specific regressions. Nice.

src/Appwrite/Platform/Workers/Functions.php (1)

416-418: LGTM: single-source execution ID when creating a new execution

Generating a new $executionId only when the document is missing and immediately reflecting it into the header keeps the ID consistent between the header and the persisted Execution.