Skip to content

feat(auth): harden credential storage and secure tui bearer input#28

Closed
alDuncanson wants to merge 4 commits intomainfrom
credential-hardening
Closed

feat(auth): harden credential storage and secure tui bearer input#28
alDuncanson wants to merge 4 commits intomainfrom
credential-hardening

Conversation

@alDuncanson
Copy link
Copy Markdown
Owner

This pull request introduces major improvements to authentication handling and credential storage security in the CLI and session management modules. The CLI now supports multiple ways to provide bearer tokens, and session credentials are securely stored using the OS keyring when available, with fallback to plaintext if not. Additionally, file and directory permissions for session storage are hardened. Comprehensive tests have been added to ensure these features work correctly.

Authentication enhancements in CLI:

  • Added --bearer-command and --bearer-stdin options to the tui command, allowing bearer tokens to be supplied via a shell command or stdin, and enforced mutual exclusivity among bearer token sources.
  • Implemented input validation and error handling for bearer token sources, including subprocess failures and empty tokens.
  • Added tests covering new bearer token source options, error cases, and validation logic.

Credential storage security:

  • Integrated OS keyring support for storing session credentials, with fallback to plaintext if unavailable, including serialization/deserialization logic and robust error handling.
  • Ensured credentials are deleted from the keyring when sessions or credentials are cleared. [1] [2]
  • Added tests for keyring-backed and plaintext credential storage, credential deletion, and compatibility.

Session storage permissions:

  • Hardened session file and directory permissions to restrict access (mode 0o700 for directories, 0o600 for files), and added tests to verify enforcement. [1] [2] [3]

Dependency updates:

  • Added keyring to pyproject.toml dependencies to enable secure credential storage.

def _print(self, text: str) -> None:
"""Print text to stdout."""
print(text)
sys.stdout.write(f"{text}\n")

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.

Copilot Autofix

AI 23 days ago

General fix: Ensure that any user‑controlled string that can contain secrets is redacted before being written to stdout, both for plain text and for structured (JSON/NDJSON) output. The safest place is at the boundary: _print should not emit raw text that may contain secrets; instead, it should run text through _redact_text. For structured output, we already call _redact_data before serializing, so that path is safe.

Best targeted fix without changing functionality:

  1. Change _print so it does not accept already‑serialized JSON/NDJSON; instead, do redaction before serialization and then call _print with the final string. However, we already have that behavior in _emit_structured, so we only need to ensure _print is not responsible for redacting structured data.
  2. For text output, _emit_text already calls _redact_text, so no change is required there.
  3. For structured output, instead of calling _print(json_module.dumps(...)), we can keep that as is, because _redact_data has already processed the payload. The issue flagged is at _print where CodeQL sees a tainted string; we can make _print assume it may receive either already‑safe structured data or raw text and still run _redact_text over it. _redact_text is written to detect and redact secrets in arbitrary strings, so passing serialized JSON through it provides an additional safety net and resolves the static analysis complaint.

Concretely, in src/a2a_handler/common/output.py:

  • Modify _print to call _redact_text(text) before writing to stdout.
  • Leave callers (_emit_text, _emit_structured, etc.) unchanged; they still perform their existing redaction, but _print becomes a final guard, ensuring that even if a future caller passes tainted data, it gets redacted.

No changes are required in src/a2a_handler/cli/auth.py because it does not log actual secret values, only identifiers, and once the output layer is hardened, any future sensitive values would still be protected.


Suggested changeset 1
src/a2a_handler/common/output.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/a2a_handler/common/output.py b/src/a2a_handler/common/output.py
--- a/src/a2a_handler/common/output.py
+++ b/src/a2a_handler/common/output.py
@@ -144,7 +144,8 @@
 
     def _print(self, text: str) -> None:
         """Print text to stdout."""
-        sys.stdout.write(f"{text}\n")
+        safe_text = _redact_text(text)
+        sys.stdout.write(f"{safe_text}\n")
 
     def _emit_text(self, text: str, force: bool = False) -> None:
         """Print plain text output with quiet-mode handling."""
EOF
@@ -144,7 +144,8 @@

def _print(self, text: str) -> None:
"""Print text to stdout."""
sys.stdout.write(f"{text}\n")
safe_text = _redact_text(text)
sys.stdout.write(f"{safe_text}\n")

def _emit_text(self, text: str, force: bool = False) -> None:
"""Print plain text output with quiet-mode handling."""
Copilot is powered by AI and may make mistakes. Always verify output.
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.

2 participants