From 8a317f6869602ac698bf9dc758288a86f4b07718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Tue, 7 Jul 2026 14:58:57 +0200 Subject: [PATCH] docs: add security documentation for FFI trust model and API key handling (closes #69) - Add Security section to README with 5 subsections (Trust Model, Input Validation, Memory Limits, File Permissions, Supply Chain) + Known Security Limitations table (SEC-001 through SEC-012) - Update SECURITY.md with vulnerability disclosure response-time commitment, FFI security model, supply chain notes, and API key safety guidance - Update OpenAIDenseEmbedding and QwenDenseEmbedding PHPDoc to reference SECURITY.md for API key handling; examples use getenv() pattern --- CHANGELOG.md | 36 ++++++++------- README.md | 58 +++++++++++++++++++++++++ SECURITY.md | 31 ++++++++++++- src/embeddings/OpenAIDenseEmbedding.php | 11 ++++- src/embeddings/QwenDenseEmbedding.php | 11 ++++- 5 files changed, 128 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfc85e9..cc07002 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,24 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### 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 - - Proxy configuration remains as a separate `curl_setopt()` call (unchanged) - - Added static analysis test `test_embedding_ssl_verify.phpt` to verify SSL options are present in source - ### Added +- **DOC-010: Missing Security Documentation** (#69) + - Added Security section to README covering trust model, input validation, memory limits, file permissions, supply chain, and selected known security limitations (SEC-001, SEC-002, SEC-004, SEC-008, SEC-012) + - Updated `SECURITY.md` with vulnerability disclosure response-time commitment, FFI security model, supply chain notes, and API key safety guidance + - Updated `OpenAIDenseEmbedding` and `QwenDenseEmbedding` PHPDoc blocks to reference `SECURITY.md` for API key handling guidelines; code examples now use `getenv()` pattern + - See `README.md#security` and `SECURITY.md` for full documentation + - **TEST-001: ZVecException isolation tests for error code strings, constructor, chaining, and error details** (#99) - Added 4 unit test files for `ZVecException` class: - `test_exception_error_code_string.phpt` — getErrorCodeString() mapping for codes 0-10, 99, and -1 @@ -182,6 +172,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Lock is released in `finally` block — crash-safe (kernel auto-releases `flock` on process termination) - Added `tests/test_installer_flock.phpt` — 4 test scenarios covering basic flock, concurrent serialization, source code verification, and behavioral double-check test with `Installer::install()` +- **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 + - Proxy configuration remains as a separate `curl_setopt()` call (unchanged) + - Added static analysis test `test_embedding_ssl_verify.phpt` to verify SSL options are present in source + ### Changed - **SMELL-004: Triplicated Insert/Upsert/Update Code** (#85) diff --git a/README.md b/README.md index 3870c29..8c06108 100644 --- a/README.md +++ b/README.md @@ -288,12 +288,70 @@ See `tasks/done/` for detailed planning documents. ### Remaining - [ ] FP64 (double) vectors (`tasks/todo/29_fp64_vectors.md`) +## Security + +See [`SECURITY.md`](./SECURITY.md) for the full security policy, including how to report vulnerabilities +and which versions are supported. + +### Trust Model + +The FFI shared library (`libzvec_ffi.so`) is loaded into the PHP process address +space and runs with the **same system privileges** as the PHP process itself. Only +load `.so` files from trusted sources. Pre-built libraries are downloaded from +GitHub Releases with SHA-256 checksum verification. + +### Input Validation + +- **Collection paths** (`$path` in `create()`/`open()`): Should NOT come from + untrusted user input. An attacker could create or access arbitrary directories. +- **Filter expressions** (`$filter` in `query()`/`deleteByFilter()`): Passed + directly to the zvec C++ engine. Sanitize any user-controlled filter strings. + The filter language does not support SQL injection, but malformed expressions + may cause errors or unexpected behavior. +- **Document primary keys**: Passed directly to the C++ layer. Ensure PK values + are validated if sourced from user input. + +### Memory Limits + +Use `ZVec::init(memoryLimitMb: 1024)` to cap collection cache memory. Without +a memory limit, collections may consume all available system memory under heavy +write or search load. + +### File Permissions + +Collection directories inherit the OS default umask. On shared hosting, ensure +collection paths are not world-readable. Set restrictive permissions explicitly +after creation if needed: + +```bash +chmod 700 /path/to/collection +``` + +### Supply Chain + +Pre-built shared libraries are downloaded from GitHub Releases via `vendor/bin/zvec-install`. +Downloads use HTTPS with TLS verification (CA bundle). SHA-256 checksums are verified +before extraction using `hash_equals()` (timing-safe comparison). + +For production deployments, build the library from source with `./build_zvec.sh` +for full supply chain control. + ## Known Limitations - **GroupByQuery**: The C++ API has this method but it returns all documents in a single group with empty group value. This is a known issue in upstream zvec (marked as "Coming Soon" in zvec docs). - **Platform**: Pre-built FFI library available for Linux x86_64 (glibc). macOS builds coming soon. - **musl Linux** (e.g., Alpine): musl-based Linux is not yet supported by the pre-built library. Build from source instead. +### Known Security Limitations + +| ID | Description | Issue | Status | +|---|---|---|---| +| **SEC-001** | SHA-256 checksum verification added to downloaded `.so` files | [#70](https://github.com/crazy-goat/php-zvec/issues/70) | ✅ Fixed v0.4.12 | +| **SEC-002** | `tempnam()` symlink race replaced with cryptographically random temp directory | [#71](https://github.com/crazy-goat/php-zvec/issues/71) | ✅ Fixed v0.4.12 | +| **SEC-004** | Null pointer safety in C++ FFI bridge — 50+ handle-accepting functions | [#73](https://github.com/crazy-goat/php-zvec/issues/73) | 🔄 In Progress | +| **SEC-008** | API key masking in `var_dump()` + `sodium_memzero()` in destructor | [#76](https://github.com/crazy-goat/php-zvec/issues/76) | ✅ Fixed v0.4.12 | +| **SEC-012** | Explicit SSL certificate verification in embedding API requests | [#80](https://github.com/crazy-goat/php-zvec/issues/80) | ✅ Fixed v0.4.12 | + ## License Same as upstream zvec project (Apache License 2.0) diff --git a/SECURITY.md b/SECURITY.md index bd4eeaf..c9dbdc8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -8,7 +8,36 @@ ## Reporting a Vulnerability -Please open a GitHub issue with the `type:security` label, or contact the maintainers directly. +Please open a [GitHub Security Advisory](https://github.com/crazy-goat/php-zvec/security/advisories/new) +or contact the maintainers directly. You will receive a response within 48 hours. + +## FFI Security Model + +This PHP library uses FFI to load a native shared library (`libzvec_ffi.so`). +The shared library runs with the **same system privileges as the PHP process**. +Only load trusted `.so` files from official sources. + +## Supply Chain + +Pre-built shared libraries are downloaded from GitHub Releases over HTTPS. +SHA-256 checksum verification is performed before extraction (see SEC-001 below). +For maximum trust, build the library from source using `./build_zvec.sh`. + +## API Key Safety + +Embedding classes accept API keys as constructor parameters. Best practice: +use environment variables: + +```php +// Recommended: +$apiKey = getenv('OPENAI_API_KEY'); +$embedding = new OpenAIDenseEmbedding(apiKey: $apiKey); + +// NOT recommended: +$embedding = new OpenAIDenseEmbedding(apiKey: 'sk-...hardcoded...'); +``` + +See the [Security section in README](./README.md#security) for more details. ## Security Mitigations diff --git a/src/embeddings/OpenAIDenseEmbedding.php b/src/embeddings/OpenAIDenseEmbedding.php index 3784935..1f4bfa6 100644 --- a/src/embeddings/OpenAIDenseEmbedding.php +++ b/src/embeddings/OpenAIDenseEmbedding.php @@ -13,16 +13,25 @@ * * Supports text-embedding-3-small, text-embedding-3-large, and text-embedding-ada-002 models. * + * API key security: + * - Use environment variables (getenv()) instead of hardcoding keys + * - See README Security section and SECURITY.md for API key safety guidance + * - The API key is masked in var_dump() output and cleared from memory + * via sodium_memzero() in __destruct() when ext-sodium is available + * * Example usage: * ```php + * // Recommended: use environment variable * $embedder = new OpenAIDenseEmbedding( - * apiKey: 'sk-...', + * apiKey: getenv('OPENAI_API_KEY'), * model: 'text-embedding-3-small' * ); * * $vector = $embedder->embed('Hello world'); * // Returns float[1536] for text-embedding-3-small * ``` + * + * @see SECURITY.md for full security policy and API key handling guidelines */ class OpenAIDenseEmbedding extends ApiEmbeddingFunction implements DenseEmbeddingFunction { diff --git a/src/embeddings/QwenDenseEmbedding.php b/src/embeddings/QwenDenseEmbedding.php index 2598635..020bf55 100644 --- a/src/embeddings/QwenDenseEmbedding.php +++ b/src/embeddings/QwenDenseEmbedding.php @@ -13,15 +13,24 @@ * * Supports text-embedding-v4 and other DashScope text embedding models. * + * API key security: + * - Use environment variables (getenv()) instead of hardcoding keys + * - See README Security section and SECURITY.md for API key safety guidance + * - The API key is masked in var_dump() output and cleared from memory + * via sodium_memzero() in __destruct() when ext-sodium is available + * * Example usage: * ```php + * // Recommended: use environment variable * $embedder = new QwenDenseEmbedding( - * apiKey: 'sk-...', + * apiKey: getenv('DASHSCOPE_API_KEY'), * model: 'text-embedding-v4' * ); * * $vector = $embedder->embed('Hello world'); * ``` + * + * @see SECURITY.md for full security policy and API key handling guidelines */ class QwenDenseEmbedding extends ApiEmbeddingFunction implements DenseEmbeddingFunction {