diff --git a/CHANGELOG.md b/CHANGELOG.md index 475a75d..38920f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- **SEC-008: Mask API keys in debug output and clear from memory in destructor** (#76) + - Added `__debugInfo()` method to `ApiEmbeddingFunction` — masks API key as `***xxxx` (last 4 chars visible) in `var_dump()` output + - Added `__destruct()` method to `ApiEmbeddingFunction` — calls `sodium_memzero()` on the API key string buffer when `ext-sodium` is available + - Constructor `$apiKey` parameter changed from `string` to `?string = null` — falls back to `OPENAI_API_KEY` or `DASHSCOPE_API_KEY` environment variables + - Added `private __clone()` to prevent cloned-instance string buffer corruption + - Updated `SECURITY.md` — marked "API keys in memory" as fixed + - Added 4 test files: `test_embedding_apikey_mask.phpt` (source analysis), `test_embedding_apikey_env.phpt` (env var fallback), `test_embedding_apikey_destruct.phpt` (destructor), `test_embedding_apikey_runtime.phpt` (runtime validation) + - **SEC-012: Enforce explicit SSL certificate verification in embedding API requests** (#80) - Added `CURLOPT_SSL_VERIFYPEER => true` and `CURLOPT_SSL_VERIFYHOST => 2` to all embedding HTTP requests - Using `curl_setopt_array()` to ensure SSL options are always applied together diff --git a/SECURITY.md b/SECURITY.md index 01dc84b..bd4eeaf 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -55,4 +55,4 @@ All C++ FFI functions accept opaque handle types. When these are null (e.g., aft 1. **Local filesystem security**: The temp directory fix (SEC-002) assumes the attacker already has local user access. It prevents privilege escalation via symlink attacks. 2. **TLS is not explicitly pinned**: The download stream context sets `verify_peer` and `verify_peer_name` to true, relying on the system's CA bundle. No certificate pinning is implemented. -3. **API keys in memory**: Embedding function API keys are stored in PHP string properties and are not explicitly zeroed after use. This is a known limitation (see SEC-008). +3. **API keys in memory**: Embedding function API keys are stored in PHP string properties. Starting from v0.4.12+, `__debugInfo()` masks the key in var_dump output and `__destruct()` calls `sodium_memzero()` when the sodium extension is available (see SEC-008). diff --git a/src/embeddings/EmbeddingInterfaces.php b/src/embeddings/EmbeddingInterfaces.php index 7c4ecb3..5a29e38 100644 --- a/src/embeddings/EmbeddingInterfaces.php +++ b/src/embeddings/EmbeddingInterfaces.php @@ -73,23 +73,56 @@ public function embedBatch(array $inputs): array; */ abstract class ApiEmbeddingFunction { - protected string $apiKey; + protected string $apiKey = ''; protected string $baseUrl; protected int $timeout; protected ?string $proxy = null; public function __construct( - string $apiKey, + ?string $apiKey = null, ?string $baseUrl = null, int $timeout = 30, ?string $proxy = null ) { - $this->apiKey = $apiKey; + $this->apiKey = $apiKey ?? (getenv('OPENAI_API_KEY') ?: null) ?? (getenv('DASHSCOPE_API_KEY') ?: null) ?? ''; $this->baseUrl = $baseUrl ?? $this->getDefaultBaseUrl(); $this->timeout = $timeout; $this->proxy = $proxy; } + /** + * Control var_dump() output — mask the API key to prevent accidental exposure. + * + * @return array Debug info with masked API key + */ + public function __debugInfo(): array + { + return [ + 'apiKey' => '***' . substr($this->apiKey, -4), + 'baseUrl' => $this->baseUrl, + 'timeout' => $this->timeout, + 'proxy' => $this->proxy, + ]; + } + + /** + * Destructor — clear API key from memory if sodium extension is available. + */ + public function __destruct() + { + if (function_exists('sodium_memzero')) { + sodium_memzero($this->apiKey); + } + } + + /** + * Prevent cloning — cloned instances would share the same string buffer + * for $apiKey, and sodium_memzero() in __destruct would corrupt the clone's key. + */ + private function __clone(): void + { + } + abstract protected function getDefaultBaseUrl(): string; /** diff --git a/src/embeddings/OpenAIDenseEmbedding.php b/src/embeddings/OpenAIDenseEmbedding.php index 455e0fe..3784935 100644 --- a/src/embeddings/OpenAIDenseEmbedding.php +++ b/src/embeddings/OpenAIDenseEmbedding.php @@ -45,7 +45,7 @@ class OpenAIDenseEmbedding extends ApiEmbeddingFunction implements DenseEmbeddin /** * Constructor. * - * @param string $apiKey OpenAI API key + * @param string|null $apiKey OpenAI API key (default: OPENAI_API_KEY env var) * @param string $model Model name (default: text-embedding-3-small) * @param int|null $dimensions Number of dimensions (optional, only for v3 models) * @param string|null $baseUrl Custom base URL (default: https://api.openai.com/v1) @@ -53,7 +53,7 @@ class OpenAIDenseEmbedding extends ApiEmbeddingFunction implements DenseEmbeddin * @param string|null $proxy HTTP proxy URL (optional) */ public function __construct( - string $apiKey, + ?string $apiKey = null, string $model = self::MODEL_SMALL, ?int $dimensions = null, ?string $baseUrl = null, diff --git a/src/embeddings/QwenDenseEmbedding.php b/src/embeddings/QwenDenseEmbedding.php index af95ed9..2598635 100644 --- a/src/embeddings/QwenDenseEmbedding.php +++ b/src/embeddings/QwenDenseEmbedding.php @@ -45,14 +45,14 @@ class QwenDenseEmbedding extends ApiEmbeddingFunction implements DenseEmbeddingF /** * Constructor. * - * @param string $apiKey DashScope API key + * @param string|null $apiKey DashScope API key (default: DASHSCOPE_API_KEY env var) * @param string $model Model name (default: text-embedding-v4) * @param string|null $baseUrl Custom base URL (default: https://dashscope.aliyuncs.com/api/v1) * @param int $timeout Request timeout in seconds (default: 30) * @param string|null $proxy HTTP proxy URL (optional) */ public function __construct( - string $apiKey, + ?string $apiKey = null, string $model = self::MODEL_V4, ?string $baseUrl = null, int $timeout = 30, diff --git a/tests/test_embedding_apikey_destruct.phpt b/tests/test_embedding_apikey_destruct.phpt new file mode 100644 index 0000000..68da3b9 --- /dev/null +++ b/tests/test_embedding_apikey_destruct.phpt @@ -0,0 +1,59 @@ +--TEST-- +SEC-008: API key destructor — __destruct with sodium_memzero +--FILE-- +apiKey)')) { + echo "PASS: __destruct calls sodium_memzero on apiKey\n"; +} else { + echo "FAIL: sodium_memzero call not found in __destruct\n"; + exit(1); +} + +// Test 3: function_exists check for sodium_memzero +if (str_contains($source, 'function_exists(\'sodium_memzero\')')) { + echo "PASS: __destruct checks if sodium_memzero exists before calling\n"; +} else { + echo "FAIL: function_exists check for sodium_memzero not found\n"; + exit(1); +} + +// Test 4: Verify the destructor is in the ApiEmbeddingFunction class +$apiClassStart = strpos($source, 'abstract class ApiEmbeddingFunction'); +$destructPos = strpos($source, 'function __destruct()'); +if ($destructPos !== false && $apiClassStart !== false && $destructPos > $apiClassStart) { + echo "PASS: __destruct method is inside ApiEmbeddingFunction class\n"; +} else { + echo "FAIL: __destruct not found inside ApiEmbeddingFunction class\n"; + exit(1); +} + +// Test 5: Verify __debugInfo is also inside ApiEmbeddingFunction +$debugInfoPos = strpos($source, 'function __debugInfo()'); +if ($debugInfoPos !== false && $debugInfoPos > $apiClassStart) { + echo "PASS: __debugInfo method is inside ApiEmbeddingFunction class\n"; +} else { + echo "FAIL: __debugInfo not found inside ApiEmbeddingFunction class\n"; + exit(1); +} + +echo "\nAll destructor source checks passed!\n"; +?> +--EXPECT-- +PASS: __destruct method declared in EmbeddingInterfaces.php +PASS: __destruct calls sodium_memzero on apiKey +PASS: __destruct checks if sodium_memzero exists before calling +PASS: __destruct method is inside ApiEmbeddingFunction class +PASS: __debugInfo method is inside ApiEmbeddingFunction class + +All destructor source checks passed! diff --git a/tests/test_embedding_apikey_env.phpt b/tests/test_embedding_apikey_env.phpt new file mode 100644 index 0000000..34e1b42 --- /dev/null +++ b/tests/test_embedding_apikey_env.phpt @@ -0,0 +1,67 @@ +--TEST-- +SEC-008: API key loading from environment variables — source analysis +--FILE-- +apiKey\s*=\s*\$apiKey\s*\?\?/', $constructBody); + if ($hasCoalescing) { + echo "PASS: Constructor uses null coalescing for apiKey with env var fallback\n"; + } else { + echo "FAIL: Null coalescing pattern not found in constructor\n"; + exit(1); + } +} else { + echo "FAIL: Could not locate constructor\n"; + exit(1); +} + +// Test 4: OpenAIDenseEmbedding constructor also nullable +$openaiSource = file_get_contents(__DIR__ . '/../src/embeddings/OpenAIDenseEmbedding.php'); +if (str_contains($openaiSource, '?string $apiKey = null')) { + echo "PASS: OpenAIDenseEmbedding constructor has nullable apiKey\n"; +} else { + echo "FAIL: OpenAIDenseEmbedding constructor apiKey not nullable\n"; + exit(1); +} + +// Test 5: QwenDenseEmbedding constructor also nullable +$qwenSource = file_get_contents(__DIR__ . '/../src/embeddings/QwenDenseEmbedding.php'); +if (str_contains($qwenSource, '?string $apiKey = null')) { + echo "PASS: QwenDenseEmbedding constructor has nullable apiKey\n"; +} else { + echo "FAIL: QwenDenseEmbedding constructor apiKey not nullable\n"; + exit(1); +} + +echo "\nAll API key env var source checks passed!\n"; +?> +--EXPECT-- +PASS: Constructor uses OPENAI_API_KEY and DASHSCOPE_API_KEY env vars as fallback +PASS: apiKey constructor parameter is nullable with default null +PASS: Constructor uses null coalescing for apiKey with env var fallback +PASS: OpenAIDenseEmbedding constructor has nullable apiKey +PASS: QwenDenseEmbedding constructor has nullable apiKey + +All API key env var source checks passed! diff --git a/tests/test_embedding_apikey_mask.phpt b/tests/test_embedding_apikey_mask.phpt new file mode 100644 index 0000000..3bad586 --- /dev/null +++ b/tests/test_embedding_apikey_mask.phpt @@ -0,0 +1,72 @@ +--TEST-- +SEC-008: API key masking — __debugInfo() source analysis +--FILE-- +apiKey, -4)")) { echo "FAIL: API key mask pattern not found\n"; exit(1); } +echo "PASS: API key mask pattern found (*** + last 4 chars)\n"; + +// apiKey key in return array +if (!str_contains($source, "'apiKey'")) { echo "FAIL: apiKey key not in __debugInfo return array\n"; exit(1); } +echo "PASS: apiKey key present in __debugInfo return array\n"; + +// Raw apiKey NOT exposed directly +$debugInfoPos = strpos($source, 'function __debugInfo()'); +$braceCount = 0; +$debugBody = ''; +for ($i = $debugInfoPos; $i < strlen($source); $i++) { + $debugBody .= $source[$i]; + if ($source[$i] === '{') $braceCount++; + if ($source[$i] === '}') $braceCount--; + if ($braceCount === 0 && $i > $debugInfoPos) break; +} +if (str_contains($debugBody, "'apiKey' => \$this->apiKey")) { echo "FAIL: Raw apiKey exposed in __debugInfo\n"; exit(1); } +echo "PASS: Raw apiKey not exposed directly in __debugInfo\n"; + +// __clone method prevents cloned-instance buffer corruption +if (!str_contains($source, 'function __clone()')) { echo "FAIL: __clone method not found\n"; exit(1); } +echo "PASS: __clone method declared to prevent cloning\n"; + +// Destructor with sodium_memzero +if (!str_contains($source, 'sodium_memzero($this->apiKey)')) { echo "FAIL: sodium_memzero call not found\n"; exit(1); } +echo "PASS: Destructor calls sodium_memzero on apiKey\n"; +if (!str_contains($source, "function_exists('sodium_memzero')")) { echo "FAIL: function_exists guard not found\n"; exit(1); } +echo "PASS: Destructor checks sodium_memzero exists before calling\n"; + +// Nullable apiKey with env var fallback +if (!str_contains($source, '?string $apiKey = null')) { echo "FAIL: Nullable apiKey not found\n"; exit(1); } +if (!str_contains($source, "getenv('OPENAI_API_KEY')")) { echo "FAIL: OPENAI_API_KEY env var fallback not found\n"; exit(1); } +if (!str_contains($source, "getenv('DASHSCOPE_API_KEY')")) { echo "FAIL: DASHSCOPE_API_KEY env var fallback not found\n"; exit(1); } +echo "PASS: Constructor has nullable apiKey with env var fallback\n"; + +// Verify OpenAIDenseEmbedding constructor is also nullable +$openaiSource = file_get_contents(__DIR__ . '/../src/embeddings/OpenAIDenseEmbedding.php'); +if (!str_contains($openaiSource, '?string $apiKey = null')) { echo "FAIL: OpenAIDenseEmbedding apiKey not nullable\n"; exit(1); } +echo "PASS: OpenAIDenseEmbedding constructor has nullable apiKey\n"; + +// Verify QwenDenseEmbedding constructor is also nullable +$qwenSource = file_get_contents(__DIR__ . '/../src/embeddings/QwenDenseEmbedding.php'); +if (!str_contains($qwenSource, '?string $apiKey = null')) { echo "FAIL: QwenDenseEmbedding apiKey not nullable\n"; exit(1); } +echo "PASS: QwenDenseEmbedding constructor has nullable apiKey\n"; + +echo "\nAll API key security source checks passed!\n"; +?> +--EXPECT-- +PASS: __debugInfo method declared in EmbeddingInterfaces.php +PASS: API key mask pattern found (*** + last 4 chars) +PASS: apiKey key present in __debugInfo return array +PASS: Raw apiKey not exposed directly in __debugInfo +PASS: __clone method declared to prevent cloning +PASS: Destructor calls sodium_memzero on apiKey +PASS: Destructor checks sodium_memzero exists before calling +PASS: Constructor has nullable apiKey with env var fallback +PASS: OpenAIDenseEmbedding constructor has nullable apiKey +PASS: QwenDenseEmbedding constructor has nullable apiKey + +All API key security source checks passed! diff --git a/tests/test_embedding_apikey_runtime.phpt b/tests/test_embedding_apikey_runtime.phpt new file mode 100644 index 0000000..4a3ec61 --- /dev/null +++ b/tests/test_embedding_apikey_runtime.phpt @@ -0,0 +1,122 @@ +--TEST-- +SEC-008: API key masking — runtime __debugInfo() and clone protection +--SKIPIF-- +hasMethod('__debugInfo')) { + die('skip zvec extension loaded — extension class lacks __debugInfo'); + } +} +?> +--FILE-- +__debugInfo(); +if ($debug['apiKey'] !== '***2345') { + echo "FAIL: __debugInfo apiKey expected '***2345', got '" . $debug['apiKey'] . "'\n"; + exit(1); +} +if (!isset($debug['baseUrl'])) { + echo "FAIL: __debugInfo missing baseUrl\n"; + exit(1); +} +if (!isset($debug['timeout'])) { + echo "FAIL: __debugInfo missing timeout\n"; + exit(1); +} +if (!array_key_exists('proxy', $debug)) { + echo "FAIL: __debugInfo missing proxy\n"; + exit(1); +} +echo "PASS: __debugInfo returns masked apiKey + baseUrl + timeout + proxy\n"; + +// Test 3: clone is prevented (sodium_memzero shared buffer protection) +$cloneError = ''; +try { + $clone = clone $e; + echo "FAIL: clone should throw Error\n"; + exit(1); +} catch (\Error $err) { + $cloneError = $err->getMessage(); + if (!str_contains($cloneError, 'clone')) { + echo "FAIL: clone error unexpected: " . $cloneError . "\n"; + exit(1); + } + echo "PASS: clone() throws Error\n"; +} + +// Test 4: Empty key shows *** (substr('', -4) returns '') +$e2 = new OpenAIDenseEmbedding(''); +$debug2 = $e2->__debugInfo(); +if ($debug2['apiKey'] !== '***') { + echo "FAIL: Empty key not masked correctly, expected '***', got '" . $debug2['apiKey'] . "'\n"; + exit(1); +} +echo "PASS: Empty API key shows '***'\n"; + +// Test 5: Env var loading (OPENAI_API_KEY) +putenv('OPENAI_API_KEY=sk-env-test-key'); +$e3 = new OpenAIDenseEmbedding(); +$debug3 = $e3->__debugInfo(); +if (!str_contains($debug3['apiKey'], '***-key')) { + echo "FAIL: API key not loaded from OPENAI_API_KEY env var\n"; + exit(1); +} +echo "PASS: API key loaded from OPENAI_API_KEY env var\n"; +putenv('OPENAI_API_KEY'); + +// Test 6: Explicit key takes priority over env var +putenv('OPENAI_API_KEY=sk-env-wrong'); +$e4 = new OpenAIDenseEmbedding('sk-explicit-correct'); +$debug4 = $e4->__debugInfo(); +if ($debug4['apiKey'] !== '***rect') { + echo "FAIL: Explicit key should override env var\n"; + exit(1); +} +echo "PASS: Explicit API key takes priority over env var\n"; +putenv('OPENAI_API_KEY'); + +// Test 7: No key and no env var — empty string +$e5 = new OpenAIDenseEmbedding(); +$debug5 = $e5->__debugInfo(); +if ($debug5['apiKey'] !== '***') { + echo "FAIL: Expected empty key mask '***', got '" . $debug5['apiKey'] . "'\n"; + exit(1); +} +echo "PASS: No key and no env var — empty API key shows '***'\n"; + +echo "\nAll API key runtime tests passed!\n"; +?> +--EXPECT-- +PASS: var_dump masks API key (shows ***2345) +PASS: __debugInfo returns masked apiKey + baseUrl + timeout + proxy +PASS: clone() throws Error +PASS: Empty API key shows '***' +PASS: API key loaded from OPENAI_API_KEY env var +PASS: Explicit API key takes priority over env var +PASS: No key and no env var — empty API key shows '***' + +All API key runtime tests passed!