Skip to content

fix: keep ordinary fields that share a name with a secret #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ChiragAgg5k wants to merge 1 commit into from
+23 −5
Closed

fix: keep ordinary fields that share a name with a secret #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions src/mcp_server_appwrite/redaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import hashlib
from functools import cache
from typing import Any
from typing import Any, get_args

import appwrite_console.models
from pydantic import BaseModel
Expand Down Expand Up @@ -116,14 +116,26 @@ def _mask(data: dict[str, Any], key: str, fields: set[str]) -> None:

@cache
def _secret_names() -> frozenset[str]:
"""Every field name an SDK model marks secret."""
names: set[str] = set()
"""The field names SDK models mark secret, less any that is an ordinary
string on another model: ``code`` is the MFA challenge code, but also a
country's code, and matched by name it would hide both."""
secret: set[str] = set()
plain: set[str] = set()
for model in vars(appwrite_console.models).values():
if isinstance(model, type) and issubclass(model, BaseModel):
for name, field in model.model_fields.items():
if _secret(field):
names.add(field.alias or name)
return frozenset(names)
secret.add(field.alias or name)
elif _text(field.annotation):
plain.add(field.alias or name)
return frozenset(secret - plain)

@appwrite appwrite Bot Nov 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep MFA code redacted in untyped responses

_format_response_parse_error uses redact_payload for every unhydrated response, so removing code here disables its masking globally, not just for Country. If an MFA challenge response fails SDK hydration, its password-formatted one-time code is serialized verbatim into the MCP error result and may be retained in the conversation transcript.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/mcp_server_appwrite/redaction.py
Line: 131

Comment:
**Keep MFA `code` redacted in untyped responses**

`_format_response_parse_error` uses `redact_payload` for every unhydrated response, so removing `code` here disables its masking globally, not just for Country. If an MFA challenge response fails SDK hydration, its password-formatted one-time code is serialized verbatim into the MCP error result and may be retained in the conversation transcript.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

🟠 Major · security · Reply if this doesn't apply.



def _text(annotation: Any) -> bool:
"""A string, or a list or optional of one."""
if annotation is str:
return True
return any(_text(argument) for argument in get_args(annotation))


def _secret(field: FieldInfo) -> bool:
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/test_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,12 @@ def test_format_appwrite_error_returns_the_unhydrated_payload(self):
self.assertNotIn("standard_0123456789abcdef", message)
self.assertIn('"secret": "[redacted]"', message)
self.assertIn("Secrets redacted from this result: secret.", message)
# A name that is an ordinary field elsewhere (a country's code) stays.
coded = AppwriteException(
"Unable to parse response into Country: 1 validation error",
response={"name": "Germany", "code": "DE"},
)
self.assertIn('"code": "DE"', _format_appwrite_error(coded))

# Asked for, a one-time secret is not lost to a parse failure.
revealed = _format_appwrite_error(exc, tool_name="projects_create", reveal=True)
Expand Down
Loading