fix: remove credential/token logging and harden security by JakubAnderwald · Pull Request #949 · sooperset/mcp-atlassian

JakubAnderwald · 2026-02-20T16:38:30Z

Description

Remove all credential/token logging patterns across the codebase and harden security in SSL handling, file downloads, and JQL construction. Tokens, secrets, and credential material (full or partial) should never be written to logs at any level.

Changes

oauth.py: Remove token exchange payload logging (contained client_secret), remove response body logging (contained access_token/refresh_token), replace partial token logging with safe confirmation messages, remove unused pprint import
oauth_setup.py: Redact client_secret in environment variable output, replace partial access/refresh token logging with non-sensitive messages
dependencies.py: Replace partial user token logging (token ...{last 8 chars}) with token ...<redacted> in both Jira and Confluence fetcher creation
ssl.py: Remove SSL_OP_LEGACY_SERVER_CONNECT and OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION flags that unnecessarily weaken the TLS handshake; update docstring
attachments.py: Add path traversal guard to download_attachment() ensuring resolved path stays within current working directory
search.py: Sanitize JQL project filter interpolation by escaping double-quote characters in project names before interpolation

Testing

Unit tests added/updated
Integration tests passed
Manual checks performed: comprehensive grep sweep confirming zero credential logging remains (token[:, access_token[, refresh_token[, client_secret in log statements, response.text in debug logs, user_token[, pformat.*payload — all clean)

Checklist

Code follows project style guidelines (linting passes).
Tests added/updated for changes.
All tests pass locally.
Documentation updated (if needed).

- Remove OAuth token exchange payload logging containing client_secret - Remove OAuth token response body logging containing access/refresh tokens - Replace partial access/refresh token logging with safe confirmation messages - Redact client_secret value in OAuth setup environment variable output - Redact partial token values in multi-user HTTP mode log messages - Remove SSL legacy renegotiation flags (OP_LEGACY_SERVER_CONNECT, OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) - Add path traversal guard to Jira attachment download - Sanitize JQL project filter interpolation to prevent injection via double-quote characters - Remove unused pprint import from oauth.py

… path traversal guard - Wrap long raise ValueError() line to satisfy ruff-format 88-char limit - Add os.getcwd mock to test_download_attachment_success so /tmp path passes traversal check - Fix test_download_attachment_relative_path: use side_effect for abspath mock and separate os.getcwd mock to handle the second abspath call from the traversal guard

sooperset

Thanks for the security hardening work! The credential logging removal and TLS cleanup are well-targeted. Three items need attention — see inline comments.

sooperset · 2026-02-21T07:10:10Z


                # Build the project filter query part
+                # Sanitize project names to prevent JQL injection
+                projects = [p.replace('"', '\\"') for p in projects]


Blocker: JQL injection bypassable via backslash.

Escaping " without first escaping \ allows bypass. A project name ending with \ produces \" which in JQL is an escaped quote — the closing quote is consumed and injection escapes the string.

Need to escape \ before ":

projects = [p.replace('\\', '\\\\').replace('"', '\\"') for p in projects]

Fixed — now escaping backslashes before double-quotes:

projects = [p.replace("\\", "\\\\").replace("\"", "\\\"") for p in projects]

This prevents the \ + \" → \\" bypass.

sooperset · 2026-02-21T07:10:10Z


+            # Guard against path traversal
+            base_dir = os.path.abspath(os.getcwd())
+            if not target_path.startswith(base_dir):


String startswith() on resolved paths is a known footgun — /tmp/attack starts with /tmp/att if cwd happens to be /tmp/att.

Consider using Path.resolve().is_relative_to() instead:

base_dir = Path(os.getcwd()).resolve() resolved = Path(target_path).resolve() if not resolved.is_relative_to(base_dir): raise ValueError(...)

Also, download_issue_attachments has no equivalent guard on its target_dir.

Both addressed:

Replaced str.startswith() with Path.is_relative_to() in download_attachment() — avoids the false-positive prefix match issue.

Added the same is_relative_to() guard to download_issue_attachments() on target_dir, upfront before mkdir(parents=True), so callers get a clean ValueError instead of confusing per-download failures.

sooperset · 2026-02-21T07:10:10Z

            logger.info("------------------------------------------------------------")
            logger.info(f"ATLASSIAN_OAUTH_CLIENT_ID={oauth_config.client_id}")
-            logger.info(f"ATLASSIAN_OAUTH_CLIENT_SECRET={oauth_config.client_secret}")
+            logger.info("ATLASSIAN_OAUTH_CLIENT_SECRET=<redacted>")


This env-var line was correctly redacted, but client_secret is still logged in the VS Code config JSON blob further down in this file (around line 345 on main) via logger.info(vscode_json). That needs redaction too.

Good catch — replaced oauth_config.client_secret with "<redacted - set via environment variable>" in the VS Code config dict. The Docker args already pass it via -e ATLASSIAN_OAUTH_CLIENT_SECRET, so the logged JSON template does not need the actual value.

- Fix JQL injection bypass: escape backslashes before double-quotes - Replace str.startswith() with Path.is_relative_to() for path traversal check - Add path traversal guard to download_issue_attachments() - Redact client_secret in VS Code config JSON blob

- Fix ruff-format: multi-line list comprehension in search.py - Replace Path.resolve() with direct Path.is_relative_to() to avoid breaking os.path mocks in tests (resolve() internally calls os.path.isabs on Python 3.12) - Add os.getcwd mocks to download_issue_attachments tests that use /tmp/attachments paths

JakubAnderwald · 2026-02-21T20:42:15Z

Is the history of this PR basically one coding agent talking to another agent? :)

sooperset

Thorough credential removal — all major exposure paths addressed. I'll add regression tests for the path traversal guard and JQL sanitization in a follow-up. Great security work, thanks!

sooperset · 2026-02-24T10:55:23Z

Haha, you caught us! 😄 We've been getting so many great community PRs and issues that we started using AI agents to help keep up with reviews. The review quality is still validated by humans though — your security fixes were genuinely solid work. Thanks again for the contribution!

…sanitization Add tests for security hardening from PR #949: - Path traversal rejection in attachment downloads (absolute and relative paths) - Path traversal rejection in issue attachment downloads (ValueError propagation) - JQL value escaping for backslash and double-quote characters (inline logic) - Integration tests for projects filter with malicious special characters - Combined backslash+quote escaping case for quote_jql_identifier_if_needed Follow-up to PR #949. Github-Issue: #949

…sanitization (#983) Add tests for security hardening from PR #949: - Path traversal rejection in attachment downloads (absolute and relative paths) - Path traversal rejection in issue attachment downloads (ValueError propagation) - JQL value escaping for backslash and double-quote characters (inline logic) - Integration tests for projects filter with malicious special characters - Combined backslash+quote escaping case for quote_jql_identifier_if_needed Follow-up to PR #949. Github-Issue: #949

JKAW added 3 commits February 20, 2026 19:02

style: fix ruff-format lambda expression formatting

08f54e9

JakubAnderwald force-pushed the fix/security-credential-logging-remediation branch from def83d4 to 08f54e9 Compare February 20, 2026 18:03

sooperset requested changes Feb 21, 2026

View reviewed changes

JKAW added 2 commits February 21, 2026 21:34

JakubAnderwald requested a review from sooperset February 23, 2026 11:40

sooperset approved these changes Feb 24, 2026

View reviewed changes

sooperset merged commit 88917c1 into sooperset:main Feb 24, 2026
6 checks passed

sooperset mentioned this pull request Feb 24, 2026

test(jira): add security regression tests for path traversal and JQL sanitization #983

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove credential/token logging and harden security#949

fix: remove credential/token logging and harden security#949
sooperset merged 5 commits into
sooperset:mainfrom
JakubAnderwald:fix/security-credential-logging-remediation

JakubAnderwald commented Feb 20, 2026

Uh oh!

sooperset left a comment

Uh oh!

sooperset Feb 21, 2026

Uh oh!

JakubAnderwald Feb 21, 2026

Uh oh!

sooperset Feb 21, 2026

Uh oh!

JakubAnderwald Feb 21, 2026

Uh oh!

sooperset Feb 21, 2026

Uh oh!

JakubAnderwald Feb 21, 2026

Uh oh!

JakubAnderwald commented Feb 21, 2026

Uh oh!

sooperset left a comment

Uh oh!

Uh oh!

sooperset commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JakubAnderwald commented Feb 20, 2026

Description

Changes

Testing

Checklist

Uh oh!

sooperset left a comment

Choose a reason for hiding this comment

Uh oh!

sooperset Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

JakubAnderwald Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

sooperset Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

JakubAnderwald Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

sooperset Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

JakubAnderwald Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

JakubAnderwald commented Feb 21, 2026

Uh oh!

sooperset left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sooperset commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants