Skip to content

feat: support authenticated OpenAI-compatible embedding endpoints - #209

Merged
Paul-Kyle merged 3 commits into
phasespace-labs:mainfrom
Isha-Zaka:feat/embedding-endpoint-auth
Sep 15, 2026
Merged

Paul-Kyle merged 3 commits into
phasespace-labs:mainfrom
Isha-Zaka:feat/embedding-endpoint-auth

Conversation

@Isha-Zaka

Copy link
Copy Markdown
Contributor

Summary

Adds bearer authentication and configurable endpoint paths for hosted
OpenAI-compatible embedding providers.

The existing OpenAI-compatible embedding path assumes /v1/embeddings
and had no way to provide credentials, which prevents providers with a
different API root or bearer-auth requirement from being used.

Changes

  • load embedding credentials from PALINODE_EMBEDDING_API_KEY or
    PALINODE_EMBEDDING_API_KEY_FILE
  • send Authorization: Bearer only for the openai embedding dialect
  • add optional embeddings.primary.endpoint_path
  • preserve the existing bare-host and trailing /v1 behavior when
    endpoint_path is not configured
  • document hosted OpenAI-compatible providers and the environment variables
  • add coverage for auth, _FILE loading, endpoint paths, Ollama isolation,
    and secret logging

I used an optional endpoint_path so existing configurations continue to
behave as before, while hosted providers can treat url as the provider root
and append their provider-specific embedding path.

Native Ollama never receives the embedding API key.

Testing

  • pytest tests/test_embed_openai_dialect.py -v --tb=short
  • pytest tests/test_api_bearer_auth.py -v --tb=short
  • pytest -q
  • ruff check palinode/ tests/ scripts/
  • bandit -r palinode/ -ll
  • git diff --check

Closes #203

Allow the OpenAI embedding dialect to use bearer credentials from the environment and an optional provider-specific endpoint path.

Keep native Ollama unauthenticated and preserve the existing /v1/embeddings behavior when no custom endpoint path is configured.
@Paul-Kyle

Copy link
Copy Markdown
Member

Thank you — this is careful work and most of it is exactly right. I'm holding the merge on one thing, and it's a gap in the threat model rather than a mistake in what you built.

The key can still reach a log, on the error path. If a provider rejects the request with a 4xx whose body contains the key — Invalid API key: <key> is a common shape — then _request_json copies that body into the OllamaError message verbatim (body_4xx = (e.response.text or "").strip()[:300]), and _embed_local catches it at embedder.py:379, logs it at :384 with error=%r over str(e), and raises EmbeddingUnavailable at :392. The credential ends up in an emitted log line and in a propagating exception.

That warning is the part I'd point at: it already logs text_len instead of the text, precisely so logs never carry user content. The intent is right there — str(e) is just a channel it doesn't reach. I reproduced it against your head with a synthetic key and a stub transport — no provider was contacted.

test_embedding_api_key_is_not_emitted_in_logs covers a successful response, which is the path where the key was never at risk. The issue asked for the key to appear in no emitted log line, and the failure path is where that promise actually has to hold.

What I'd like before merging: redact the credential on the error path — wherever a response body is carried into an exception on a request that had an Authorization header — and a regression test that drives a credential-bearing failure all the way through the embedder wrapper, asserting the key is absent from both the log and the raised EmbeddingUnavailable. Please use a stub transport, as your existing tests do.

Everything else stands and I don't want it lost in the hold. Scoping the header structurally — load_embedding_api_key() called inside _embed_openai and nowhere else — means no configuration gives a native Ollama request an Authorization header, and your negative test asserts both halves of that with the key deliberately set. load_embedding_api_key() follows load_api_token() line for line including the unreadable-_FILE case, and your test asserts the path's absence from the log rather than only the message. And leaving _openai_embed_base_url() alone when endpoint_path is unset means every existing deployment runs the same code it runs today — the pre-existing /v1-doubling test passing unmodified is better evidence for that than a new test would have been.

Two smaller things for the same push, neither a condition: load_embedding_api_key() runs on every embed request, so with PALINODE_EMBEDDING_API_KEY_FILE that's a file read per request and one error log per request rather than one — if that was deliberate for live key rotation, say so and I'll document it as such. And SECURITY.md carries the operator-facing secret inventory; the new pair belongs there next to PALINODE_API_TOKEN.

Push onto the same branch and I'll review and merge once checks are green.

Sanitize authenticated provider response bodies before carrying them into embedding exceptions, add a regression test through EmbeddingUnavailable, and document the embedding credential variables.
@Isha-Zaka

Copy link
Copy Markdown
Contributor Author

@Paul-Kyle Thanks for catching this. I pushed a follow-up that redacts the credential from authenticated provider response bodies before they are carried into exceptions, including the 4xx and deterministic 5xx paths.

I also added a stub-transport regression test that drives a 401 response echoing the key through the embedder wrapper and verifies the key is absent from both the logs and the raised EmbeddingUnavailable.
Also documented the new embedding credential pair in SECURITY.md

For the _FILE behavior, the per-request resolution is intentional so file-backed credentials can be rotated without requiring a restart.

All checks are green now.

@Paul-Kyle
Paul-Kyle merged commit d4379d8 into phasespace-labs:main Sep 15, 2026
13 checks passed
@Paul-Kyle

Copy link
Copy Markdown
Member

Merged — thank you for addressing the redaction gap and adding the regression test.

Two details worth naming. You redact before truncating to 300 characters; the other order looks identical in review and leaves a key fragment whenever the credential straddles the boundary. And your regression drives a credential-bearing 401 through the embedder wrapper rather than the client, then asserts [REDACTED] is present as well as the secret being absent — a test that only checks absence passes just as happily when the request never happened.

On per-request resolution of PALINODE_EMBEDDING_API_KEY_FILE: that is a good reason, so it stays, and it is now documented as intentional rather than looking like an oversight to whoever reads it next.

One limitation for the record, not a request: redaction matches the credential as a literal string, so a provider that echoes it re-encoded or partially masked would slip through. That is the right tradeoff for the failure we actually have, and worth knowing before anyone treats it as a guarantee.

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.

embeddings: support bearer-authenticated OpenAI-compatible endpoints with configurable paths

2 participants