fix(agent-tool-registry): harden rate limiting and api key identifiers

Co-Authored-By: Virgil <virgil@lethean.io>

php/Services/AgentToolRegistry.php

@@ -117,7 +117,7 @@ class AgentToolRegistry
      */
     public function forApiKey(ApiKey $apiKey): Collection
     {
-        $cacheKey = $this->apiKeyCacheKey($apiKey->getKey());
+        $cacheKey = $this->apiKeyCacheKey($this->apiKeyIdentifier($apiKey));
 
         $permittedNames = Cache::remember($cacheKey, self::CACHE_TTL, function () use ($apiKey) {
             return $this->all()->filter(function (AgentToolInterface $tool) use ($apiKey) {
@@ -214,8 +214,7 @@ class AgentToolRegistry
                 );
             }
 
-            $this->assertApiKeyWithinExecutionRateLimit($apiKey, $name);
-            $this->recordApiKeyExecution($apiKey);
+            $this->enforceAndRecordRateLimit($apiKey, $name);
         }
 
         // Dependency check
@@ -291,6 +290,11 @@ class AgentToolRegistry
 
     /**
      * Return a stable identifier for cache keys.
+     *
+     * ApiKey::getKey() must return a scalar or null. Non-scalar values are
+     * rejected because they are not stable across requests.
+     *
+     * @throws \InvalidArgumentException
      */
     private function apiKeyIdentifier(ApiKey $apiKey): string
     {
@@ -300,7 +304,11 @@ class AgentToolRegistry
             return (string) $identifier;
         }
 
-        return (string) spl_object_id($apiKey);
+        throw new \InvalidArgumentException(sprintf(
+            'ApiKey %s::getKey() must return a scalar or null; returned %s',
+            $apiKey::class,
+            get_debug_type($identifier)
+        ));
     }
 
     /**
@@ -328,17 +336,10 @@ class AgentToolRegistry
     }
 
     /**
-     * Get the current execution count for an API key.
+     * Ensure the API key still has execution budget for the tool call, and
+     * record the execution in one cache-backed operation.
      */
-    private function apiKeyExecutionCount(ApiKey $apiKey): int
-    {
-        return (int) Cache::get($this->executionRateCacheKey($apiKey), 0);
-    }
-
-    /**
-     * Ensure the API key still has execution budget for the tool call.
-     */
-    private function assertApiKeyWithinExecutionRateLimit(ApiKey $apiKey, string $toolName): void
+    private function enforceAndRecordRateLimit(ApiKey $apiKey, string $toolName): void
     {
         $rateLimit = $this->apiKeyExecutionRateLimit($apiKey);
 
@@ -346,22 +347,19 @@ class AgentToolRegistry
             return;
         }
 
-        if ($this->apiKeyExecutionCount($apiKey) >= $rateLimit) {
+        $cacheKey = $this->executionRateCacheKey($apiKey);
+        $count = 1;
+
+        if (! Cache::add($cacheKey, $count, self::EXECUTION_RATE_LIMIT_CACHE_TTL)) {
+            $count = (int) Cache::increment($cacheKey);
+        }
+
+        if ($count > $rateLimit) {
+            Cache::decrement($cacheKey);
+
             throw new \RuntimeException(
                 "Rate limit exceeded: API key cannot execute tool '{$toolName}' right now"
             );
         }
     }
-
-    /**
-     * Record a tool execution in the cache budget.
-     */
-    private function recordApiKeyExecution(ApiKey $apiKey): void
-    {
-        $cacheKey = $this->executionRateCacheKey($apiKey);
-
-        if (! Cache::add($cacheKey, 1, self::EXECUTION_RATE_LIMIT_CACHE_TTL)) {
-            Cache::increment($cacheKey);
-        }
-    }
 }

php/tests/Unit/AgentToolRegistryTest.php

@@ -70,9 +70,17 @@ function makeTool(string $name, array $scopes = [], string $category = 'test'):
  * since the php-api package is not available in this test environment.
  */
 function makeApiKey(int $id, array $scopes = [], ?array $toolScopes = null, ?int $rateLimit = null): ApiKey
+{
+    return makeApiKeyWithIdentifier($id, $scopes, $toolScopes, $rateLimit);
+}
+
+/**
+ * Build a minimal ApiKey mock with a configurable identifier.
+ */
+function makeApiKeyWithIdentifier(mixed $identifier, array $scopes = [], ?array $toolScopes = null, ?int $rateLimit = null): ApiKey
 {
     $key = Mockery::mock(ApiKey::class);
-    $key->shouldReceive('getKey')->andReturn($id);
+    $key->shouldReceive('getKey')->andReturn($identifier);
     $key->shouldReceive('hasScope')->andReturnUsing(
         fn (string $scope) => in_array($scope, $scopes, true)
     );
@@ -320,4 +328,14 @@ describe('execute rate limiting', function () {
         expect(fn () => $registry->execute('plan.create', [], [], $apiKey, false))
             ->toThrow(\RuntimeException::class, 'Rate limit exceeded');
     });
+
+    it('rejects non-scalar api key identifiers', function () {
+        $registry = new AgentToolRegistry;
+        $registry->register(makeTool('plan.create', ['plans.write']));
+
+        $apiKey = makeApiKeyWithIdentifier(new stdClass, ['plans.write'], null, 1);
+
+        expect(fn () => $registry->execute('plan.create', [], [], $apiKey, false))
+            ->toThrow(\InvalidArgumentException::class, 'getKey() must return a scalar or null');
+    });
 });

tests/cli/extract/.core/workspace/test-extract/.core/reference/cli.go

@@ -107,14 +107,14 @@ func (cl *Cli) Run(args ...string) Result {
 				opts.Set("_arg", arg)
 			}
 			argsResult := opts.Get("_args")
-			args := []string{}
+			resultArgs := []string{}
 			if argsResult.OK {
 				if existing, ok := argsResult.Value.([]string); ok {
-					args = append(args, existing...)
+					resultArgs = append(resultArgs, existing...)
 				}
 			}
-			args = append(args, arg)
-			opts.Set("_args", args)
+			resultArgs = append(resultArgs, arg)
+			opts.Set("_args", resultArgs)
 		}
 	}