fix(templates): unwrap SecretStr in configurations dump (create/replace handlers)#744
Merged
swaroopvarma1 merged 1 commit intoreleasefrom May 6, 2026
Merged
Conversation
The create + replace handlers called
``template_data.configurations.model_dump(exclude_none=True)`` which
returns a Python dict with ``SecretStr`` fields (e.g.
``configurations.mcp.servers[*].auth.token``) preserved as
``SecretStr`` *instances*. The downstream ``json.dumps`` in
``create_template`` / ``replace_template`` then raised
``TypeError: Object of type SecretStr is not JSON serializable``,
which the accessor swallowed and returned ``None``, surfacing as a
generic ``500 Failed to create template``.
Fix: pass ``mode="json"`` so Pydantic unwraps SecretStr to its string
value (the literal ``{exa_api_key}`` placeholder, not a real secret —
placeholders resolve at chat-turn time from credentials).
Verified end-to-end:
- ``create_template_handler`` with the real Acme/MCP payload → 201
- Real ``curl POST /templates`` against local server (uvicorn-reloaded
with the fix) → 201, template created and roundtrips cleanly.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes template create/replace API handlers so templates whose configurations contain SecretStr fields (notably MCP auth fields like mcp.servers[*].auth.token) can be persisted without json.dumps() failing. It updates the Breeze Buddy templates handlers to dump configurations using Pydantic’s JSON serialization mode, ensuring the resulting dict is JSON-compatible for the DB accessor layer.
Changes:
- Serialize
template_data.configurationswithmodel_dump(exclude_none=True, mode="json")increate_template_handler. - Apply the same JSON-mode serialization in
replace_template_handler. - Add inline comments clarifying why JSON-mode dumping is required for
SecretStrfields.
6 tasks
swaroopvarma1
pushed a commit
that referenced
this pull request
May 6, 2026
…sponses PR #744 stopped the SecretStr-not-JSON-serializable crash on POST/PUT /templates by switching the configurations dump to mode="json". That stops the crash, but Pydantic's default JSON serialization for SecretStr is the masked form ("**********") — silently corrupting the persisted token. At call time the loaded template then sends `Authorization: Bearer **********` to the upstream MCP / HTTP endpoint, which fails authentication. Reproduced on prod with template_id 12d3bed6-fa45-45c6-b1ed-a285f57ad289 (Exa MCP, bearer token = "{exa_api_key}" placeholder, stored as "**********"). That row has been deleted. Fix: add a field_serializer on HttpAuthConfig that reveals the underlying string for the three SecretStr fields (token, password, api_key_value) **only when** model_dump is invoked with context={"reveal_secrets": True}. Without the context flag the serializer falls back to the masked "**********" form, so: - The two persistence paths (create_template_handler, replace_template_handler) pass the context and write the real value (typically a `{credential_name}` placeholder resolved per call from the credentials table). - All other JSON serialization paths — notably FastAPI response encoding for GET /templates/{id} and the PUT response — see no context, fall through to the masked form, and so do not leak literal tokens that an operator may have embedded directly. This addresses the AI review note on PR #745 that an unconditional when_used="json" unmask would cause GET/PUT response handlers (response_model=TemplateModel) to start returning raw auth material that previously stayed masked by SecretStr's default JSON behavior. mask_template_secrets() only masks template.secrets, not template.configurations, so the Pydantic-level mask was the only guard for configurations.mcp.servers[*].auth.token in API responses. The context flag preserves that guard. mode="python" (the default) keeps the SecretStr instance, so in-memory access via `.get_secret_value()` on the runtime path is unaffected. Verified end-to-end against the running local server, four cases: 1. Placeholder template: POST -> DB has "{exa_api_key}" literal, GET API response auth.token = "**********". 2. Literal-token template: POST with token="sk-LITERAL-LEAK-CANARY-12345" -> DB has the literal canary, GET API response auth.token = "**********", canary string occurs zero times in the response body (full-body grep). 3. PUT replace path: PUT with token="sk-PUT-CANARY-67890" -> DB has the new literal canary, PUT response masked, canary occurs zero times in response body. 4. Both test rows cleaned up locally after verification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swaroopvarma1
pushed a commit
that referenced
this pull request
May 6, 2026
…sponses PR #744 stopped the SecretStr-not-JSON-serializable crash on POST/PUT /templates by switching the configurations dump to mode="json". That stops the crash, but Pydantic's default JSON serialization for SecretStr is the masked form ("**********") — silently corrupting the persisted token. At call time the loaded template then sends `Authorization: Bearer **********` to the upstream MCP / HTTP endpoint, which fails authentication. Fix is in two parts: 1. HttpAuthConfig.field_serializer (template/types.py) Reveals the underlying string for the three SecretStr fields (token, password, api_key_value) **only when** model_dump is invoked with context={"reveal_secrets": True}. Without the context flag the serializer falls back to the masked "**********" form. So: - Persistence paths (create_template_handler, replace_template_handler) pass the context and write the real value (typically a `{credential_name}` placeholder resolved per call from the credentials table). - All other JSON serialization paths — notably FastAPI response encoding for GET /templates/{id} and the PUT response — see no context, fall through to the masked form, and so do not leak literal tokens that an operator may have embedded directly. This addresses the AI review note on the original PR (mask_template_secrets() only masks template.secrets, not template.configurations, so the Pydantic-level mask was the only guard for configurations.mcp.servers[*].auth.token in API responses). 2. template/cache.py — also passes reveal_secrets context The Redis L2 template cache calls model_dump_json() to populate itself. Without the context flag it would serialize the auth token as "**********" and on cache HIT reconstruct the template with auth.token = SecretStr("**********") — runtime would then send `Authorization: Bearer **********` to the MCP server. Reproduced live in chat: turn 1 (cache miss) succeeded because MCP setup ran before the cache write, but turn 2+ hit the poisoned cache and returned `web_search_exa error (401): Invalid API key` from Exa. The cache is internal/private; the masking only belongs in API responses, where the lack of context preserves it. mode="python" (the default) keeps the SecretStr instance, so in-memory access via `.get_secret_value()` on the runtime path is unaffected. Verified end-to-end against the running local server, six cases: Persistence + API response (HTTP layer) 1. Placeholder template POST -> DB has "{exa_api_key}" literal, GET API response auth.token = "**********". 2. Literal-token template POST with token="sk-LITERAL-LEAK-CANARY-12345" -> DB has the literal canary, GET API response auth.token = "**********", canary string occurs zero times in the response body (full-body grep). 3. PUT replace path: PUT with token="sk-PUT-CANARY-67890" -> DB has the new literal canary, PUT response masked, canary occurs zero times in response body. 4. Test rows cleaned up. Redis cache + chat runtime (cache.py fix) 5. Busted local Redis cache for chat-direct-claude template, ran two-turn chat session via the demo session API. Both turns show `auth-fingerprint=resolved-len=43-tail=0556fe` (the real Exa key is 36 chars, so "Bearer " + 36 = 43; tail matches the real key suffix). Cache HIT (turn 2) preserves the placeholder. 6. Turn 2 prompt asked the LLM to web-search Strait of Hormuz news; web_search_exa was invoked, returned real Reuters / India trade news content with 7 cited URLs in the assistant reply; zero `Invalid API key` strings in the response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swaroopvarma1
pushed a commit
that referenced
this pull request
May 6, 2026
…sponses PR #744 stopped the SecretStr-not-JSON-serializable crash on POST/PUT /templates by switching the configurations dump to mode="json". That stops the crash, but Pydantic's default JSON serialization for SecretStr is the masked form ("**********") — silently corrupting the persisted token. At call time the loaded template then sends `Authorization: Bearer **********` to the upstream MCP / HTTP endpoint, which fails authentication. Fix is in three parts: 1. HttpAuthConfig.field_serializer (template/types.py) Reveals the underlying string for the three SecretStr fields (token, password, api_key_value) **only when** model_dump is invoked with context={"reveal_secrets": True}. Without the context flag the serializer falls back to the masked "**********" form. So: - Persistence paths (create_template_handler, replace_template_handler) pass the context and write the real value (typically a `{credential_name}` placeholder resolved per call from the credentials table). - All other JSON serialization paths — notably FastAPI response encoding for GET /templates/{id} and the PUT response — see no context, fall through to the masked form, and so do not leak literal tokens that an operator may have embedded directly. mask_template_secrets() only masks template.secrets, not template.configurations, so the Pydantic-level mask is the only guard for configurations.mcp.servers[*].auth.token in API responses. 2. template/cache.py — also passes reveal_secrets context The Redis L2 template cache calls model_dump_json() to populate itself. Without the context flag it would serialize the auth token as "**********" and on cache HIT reconstruct the template with auth.token = SecretStr("**********") — runtime would then send `Authorization: Bearer **********` to the MCP server. Reproduced live in chat: turn 1 (cache miss) succeeded because MCP setup ran before the cache write, but turn 2+ hit the poisoned cache and returned `web_search_exa error (401): Invalid API key` from Exa. The cache is internal/private; the masking only belongs in API responses. 3. merge_masked_mcp_auth (utils/secrets.py) + replace handler Closes the GET-edit-PUT regression: GET /templates returns the auth token masked, so a UI that does GET → edit-one-unrelated- field → PUT sends the masked literal back; with reveal_secrets on the persistence path that literal would land in DB and break auth on the next call. New helper walks configurations.mcp.servers[*].auth.{token,password,api_key_value} and substitutes any masked value ("**********" or "******") with the corresponding existing persisted value, paired by server name with index fallback. If a masked value has no real counterpart (e.g. a brand-new MCP server with a placeholder the caller forgot to fill, or a rename that broke the pairing), the field is cleared to null and a warning is logged — runtime then fails loudly at MCP setup rather than silently emitting `Bearer **********`. Mirrors the existing merge_secrets shape for top-level template.secrets. mode="python" (the default) keeps the SecretStr instance, so in-memory access via `.get_secret_value()` on the runtime path is unaffected. Verified end-to-end against the running local server: Persistence + API response (HTTP layer) 1. Placeholder template POST -> DB has "{exa_api_key}" literal, GET API response auth.token = "**********". 2. Literal-token template POST with token="sk-LITERAL-LEAK-CANARY-12345" -> DB has the literal canary, GET API response auth.token = "**********", canary string occurs zero times in the response body (full-body grep). 3. PUT replace path: PUT with token="sk-PUT-CANARY-67890" -> DB has the new literal canary, PUT response masked, canary occurs zero times in response body. Redis cache + chat runtime (cache.py fix) 4. Busted local Redis cache, ran two-turn chat session via the demo session API. Both turns show `auth-fingerprint=resolved-len=43-tail=0556fe` (the real Exa key suffix). Cache HIT (turn 2) preserves the placeholder. 5. Turn 2 prompt asked for Strait of Hormuz news; web_search_exa was invoked successfully, the assistant reply contains 7 cited URLs from real search results; zero `Invalid API key` strings in the response. GET-edit-PUT regression guard (merge_masked_mcp_auth) 6. CREATE with token="sk-ORIGINAL-CANARY-12345"; GET returns "**********"; PUT-back the masked GET unchanged -> DB still holds the original canary (PASS). 7. PUT with explicit token="sk-NEW-REAL-67890" -> DB updated (PASS). 8. PUT with renamed server + masked token -> index fallback preserves the prior real value (PASS). 9. PUT that adds a brand-new MCP server with masked token (no existing pair) -> field cleared to null, warning logged (PASS — runtime will fail loudly rather than silently emitting `Bearer **********`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swaroopvarma1
pushed a commit
that referenced
this pull request
May 6, 2026
…sponses PR #744 stopped the SecretStr-not-JSON-serializable crash on POST/PUT /templates by switching the configurations dump to mode="json". That stops the crash, but Pydantic's default JSON serialization for SecretStr is the masked form ("**********") — silently corrupting the persisted token. At call time the loaded template then sends `Authorization: Bearer **********` to the upstream MCP / HTTP endpoint, which fails authentication. Fix is in three parts: 1. HttpAuthConfig.field_serializer (template/types.py) Reveals the underlying string for the three SecretStr fields (token, password, api_key_value) **only when** model_dump is invoked with context={"reveal_secrets": True}. Without the context flag the serializer falls back to the masked "**********" form. So: - Persistence paths (create_template_handler, replace_template_handler) pass the context and write the real value (typically a `{credential_name}` placeholder resolved per call from the credentials table). - All other JSON serialization paths — notably FastAPI response encoding for GET /templates/{id} and the PUT response — see no context, fall through to the masked form, and so do not leak literal tokens that an operator may have embedded directly. mask_template_secrets() only masks template.secrets, not template.configurations, so the Pydantic-level mask is the only guard for configurations.mcp.servers[*].auth.token in API responses. 2. template/cache.py — also passes reveal_secrets context The Redis L2 template cache calls model_dump_json() to populate itself. Without the context flag it would serialize the auth token as "**********" and on cache HIT reconstruct the template with auth.token = SecretStr("**********") — runtime would then send `Authorization: Bearer **********` to the MCP server. Reproduced live in chat: turn 1 (cache miss) succeeded because MCP setup ran before the cache write, but turn 2+ hit the poisoned cache and returned `web_search_exa error (401): Invalid API key` from Exa. The cache is internal/private; the masking only belongs in API responses. 3. merge_masked_mcp_auth (utils/secrets.py) + replace handler Closes the GET-edit-PUT regression: GET /templates returns the auth token masked, so a UI that does GET → edit-one-unrelated- field → PUT sends the masked literal back; with reveal_secrets on the persistence path that literal would land in DB and break auth on the next call. New helper walks configurations.mcp.servers[*].auth.{token,password,api_key_value} and substitutes any masked value ("**********" or "******") with the corresponding existing persisted value, paired by server name with index fallback. If a masked value has no real counterpart (e.g. a brand-new MCP server with a placeholder the caller forgot to fill, or a rename that broke the pairing), the field is cleared to null and a warning is logged — runtime then fails loudly at MCP setup rather than silently emitting `Bearer **********`. Mirrors the existing merge_secrets shape for top-level template.secrets. mode="python" (the default) keeps the SecretStr instance, so in-memory access via `.get_secret_value()` on the runtime path is unaffected. Verified end-to-end against the running local server: Persistence + API response (HTTP layer) 1. Placeholder template POST -> DB has "{exa_api_key}" literal, GET API response auth.token = "**********". 2. Literal-token template POST with token="sk-LITERAL-LEAK-CANARY-12345" -> DB has the literal canary, GET API response auth.token = "**********", canary string occurs zero times in the response body (full-body grep). 3. PUT replace path: PUT with token="sk-PUT-CANARY-67890" -> DB has the new literal canary, PUT response masked, canary occurs zero times in response body. Redis cache + chat runtime (cache.py fix) 4. Busted local Redis cache, ran two-turn chat session via the demo session API. Both turns show `auth-fingerprint=resolved-len=43-tail=0556fe` (the real Exa key suffix). Cache HIT (turn 2) preserves the placeholder. 5. Turn 2 prompt asked for Strait of Hormuz news; web_search_exa was invoked successfully, the assistant reply contains 7 cited URLs from real search results; zero `Invalid API key` strings in the response. GET-edit-PUT regression guard (merge_masked_mcp_auth) 6. CREATE with token="sk-ORIGINAL-CANARY-12345"; GET returns "**********"; PUT-back the masked GET unchanged -> DB still holds the original canary (PASS). 7. PUT with explicit token="sk-NEW-REAL-67890" -> DB updated (PASS). 8. PUT with renamed server + masked token -> index fallback preserves the prior real value (PASS). 9. PUT that adds a brand-new MCP server with masked token (no existing pair) -> field cleared to null, warning logged (PASS — runtime will fail loudly rather than silently emitting `Bearer **********`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Templates whose `configurations` contains any `SecretStr` field — most importantly `configurations.mcp.servers[*].auth.token` introduced in #687 / #742 — couldn't be created or updated via the API. The error surfaced as a generic `500 Failed to create template` because the accessor swallows the underlying `TypeError`.
The exception under the hood:
```
Error creating template: Object of type SecretStr is not JSON serializable
```
Root cause in `templates/handlers.py:115-116` and `:407-409`:
```python
configurations = template_data.configurations.model_dump(exclude_none=True)
```
Pydantic's default `model_dump()` keeps `SecretStr` fields as `SecretStr` instances, not plain strings. The downstream `json.dumps` in `create_template` / `replace_template` then raises.
Fix
Add `mode="json"` so Pydantic emits JSON-compatible types — `SecretStr` becomes its underlying string value (the literal placeholder like `"{exa_api_key}"`, not the real secret; the placeholder resolves at chat-turn time from the credentials table).
Same change applied to both `create_template_handler` and `replace_template_handler`. Round-trip is safe: when the stored config is later read and re-validated through `ConfigurationModel`, Pydantic re-wraps the string into `SecretStr` automatically.
Why this hadn't been caught
Existing direct-mode + MCP templates (e.g. local `chat-direct-claude`) were inserted via raw SQL, bypassing this handler. The first attempt to create one through `POST /templates` is what surfaced both this and #743.
Verification (real, end-to-end)
```
{"status":"success","template_id":"a9baa747-...","message":"Template 'http-test-942141' created successfully with 0 nodes"}
HTTP 201
```
Stacking with #743
This is a sibling fix to #743 (which let direct-mode payloads past the validator). Both are needed to create the Acme/Exa-MCP demo template via the API. With both merged, `bash prod/RUNBOOK.sh` completes through the template-create step.
🤖 Generated with Claude Code