Skip to content

fix: mask API keys in debug output and clear from memory in destructor (closes #76)#159

Merged
s2x merged 2 commits into
mainfrom
fix/issue-76-apikey-plaintext-memory
Jul 7, 2026
Merged

fix: mask API keys in debug output and clear from memory in destructor (closes #76)#159
s2x merged 2 commits into
mainfrom
fix/issue-76-apikey-plaintext-memory

Conversation

@s2x

@s2x s2x commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #76

SEC-008: API Keys Stored in Plaintext Memory

Changes

  • __debugInfo() — Added to ApiEmbeddingFunction. Masks API key as ***xxxx in var_dump() output, preventing accidental exposure in logs, crash dumps, and debug output
  • __destruct() — Calls sodium_memzero($this->apiKey) when ext-sodium is available, clearing the key from process memory
  • Constructor$apiKey changed from string to ?string = null. Falls back to OPENAI_API_KEY or DASHSCOPE_API_KEY environment variables
  • __clone() — Private to prevent cloned-instance string buffer corruption (sodium_memzero on shared buffer during destruct)
  • Backward compatible — All existing call sites pass a string argument as before; nullable with default null is a safe widening
  • SECURITY.md — Updated 'API keys in memory' from 'known limitation' to 'fixed'
  • Tests — 4 new .phpt files: source analysis (mask, env, destruct) + runtime validation (skipped when native zvec extension loaded)

Testing

  • All 4 new SEC-008 tests PASS
  • Existing SSL embedding test PASS
  • No regressions in embedding interface tests (pre-existing failures unrelated)
  • No test database leftovers

Code Review

  • Passed subagent code review
  • All review comments addressed (clone guard, runtime test added, fragile boundary fixed)

closes #76)

SEC-008: API keys stored in plaintext memory

Changes:
- Add __debugInfo() to ApiEmbeddingFunction — masks API key in var_dump
- Add __destruct() with sodium_memzero() — clears key from memory
- Constructor apiKey param now nullable with env var fallback
- Add private __clone() to prevent shared buffer corruption
- Update SECURITY.md to mark as fixed
- Add 4 test files (source analysis + runtime validation)
@s2x s2x self-assigned this Jul 7, 2026
…n (refs #76)

- Add default value '' to  property to prevent fatal error when
  subclass does not call parent constructor (MockDenseEmbedding case)
- Fix getenv() false return: getenv() returns false for unset vars,
  not null — use (getenv(...) ?: null) to make null coalescing work
- Fix runtime test: clone error msg differs per PHP version, and
  substr('', -4) returns '' not '****'
@s2x
s2x merged commit 0ffb11c into main Jul 7, 2026
3 checks passed
@s2x
s2x deleted the fix/issue-76-apikey-plaintext-memory branch July 7, 2026 11:19
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.

SEC-008: API Keys Stored in Plaintext Memory

1 participant