Skip to content

Fix credential redaction and bidi sanitization gaps #8

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

Merged
alptekinege merged 1 commit into from
Aug 2, 2026
Merged

Fix credential redaction and bidi sanitization gaps #8

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
8 changes: 6 additions & 2 deletions githubfetch/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ def redact(text: object, token: str | None = None) -> str:
if token:
out = out.replace(token, "***REDACTED***")
out = _TOKEN_PATTERN.sub("***REDACTED***", out)
# Never echo an Authorization header value back to the terminal.
# Never echo an Authorization header value back to the terminal. Redact
# everything after "Authorization:" to the end of the line - scheme and
# credential together, whatever the scheme (Bearer, Basic, Digest...), so
# a redaction cannot stop at the scheme name and leak the credential, nor
# leak multi-token values like Digest parameters.
out = re.sub(
r"(?i)(authorization[\"']?\s*[:=]\s*[\"']?)(bearer|token)?\s*\S+",
r"(?i)(authorization[\"']?\s*[:=]\s*[\"']?)[^\n\r]*",
Comment on lines 67 to +68

@coderabbitai coderabbitai Bot Aug 2, 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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python - <<'PY'
import re

sample = "Authorization: \nNext: keep"
replacement = r"\1***REDACTED***"

current = re.compile(
    r"(?i)(authorization[\"']?\s*[:=]\s*[\"']?)[^\n\r]*"
)
fixed = re.compile(
    r"(?i)(authorization[\"']?[ \t]*[:=][ \t]*[\"']?)[^\n\r]*"
)

assert current.sub(replacement, sample) == "Authorization: \n***REDACTED***"
assert fixed.sub(replacement, sample) == (
    "Authorization: ***REDACTED***\nNext: keep"
)
PY

Repository: alptekinege/githubfetch

Length of output: 161

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "## File existence and line count"
if [ -f githubfetch/api.py ]; then
  wc -l githubfetch/api.py
else
  fd -a 'api.py$' . | sed 's#^\./##'
fi

echo
echo "## githubfetch/api.py outline"
ast-grep outline githubfetch/api.py || true

echo
echo "## Relevant lines 40-90"
sed -n '40,90p' githubfetch/api.py | cat -n

echo
echo "## Search for redact usages and tests"
rg -n "def redact|redact\(|Authorization|authorization|authorization" githubfetch tests . 2>/dev/null | head -200

Repository: alptekinege/githubfetch

Length of output: 7643

Keep Authorization header redaction on the same physical line.

The regex allows \s* before and after :/=, so Authorization: \nNext: keep can consume Next: keep as part of the header value and redact unrelated output. Restrict delimiter spacing to horizontal whitespace, e.g. [ \t]*.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@githubfetch/api.py` around lines 67 - 68, Update the Authorization-redaction
regex in re.sub to use horizontal whitespace only around the header delimiter,
replacing newline-permitting spacing with a space/tab character class so
redaction cannot consume subsequent physical lines.

r"\1***REDACTED***",
out,
)
Expand Down
1 change: 1 addition & 0 deletions githubfetch/sanitize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"\u202a\u202b\u202c\u202d\u202e" # LRE, RLE, PDF, LRO, RLO
"\u2060\u2061\u2062\u2063\u2064" # word joiner + invisible operators
"\u2066\u2067\u2068\u2069" # LRI, RLI, FSI, PDI
"\u061c\u08e2" # Arabic Letter Mark, Arabic Disputed End of Ayah
"\ufeff" # BOM / ZWNBSP
)

Expand Down
23 changes: 23 additions & 0 deletions tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,29 @@ def test_redact_scrubs_authorization_header():
assert "abc123xyz" not in out


def test_redact_scrubs_basic_authorization_header():
"""Any auth scheme, not just Bearer/Token, must not leak its credential."""
out = redact("Authorization: Basic dXNlcjpwYXNz")
assert "dXNlcjpwYXNz" not in out
assert "Basic" not in out


def test_redact_scrubs_arbitrary_auth_scheme():
out = redact("authorization = \"Digest realm=example, nonce=deadbeef\"")
assert "nonce=deadbeef" not in out
assert "Digest" not in out


def test_redact_preserves_text_before_header_and_other_lines():
out = redact("Error 401: Authorization: Bearer abc123 rejected")
assert "abc123" not in out
assert "Error 401:" in out
multiline = redact("GET /users HTTP/1.1\nAuthorization: Bearer abc123\nAccept: */*")
assert "abc123" not in multiline
assert "GET /users HTTP/1.1" in multiline
assert "Accept: */*" in multiline
Comment on lines +77 to +90

@coderabbitai coderabbitai Bot Aug 2, 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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Assert removal of the complete authorization value.

The Digest test does not assert that realm=example is removed. The single-line test does not assert that trailing text such as rejected is removed. Add these assertions, or assert the exact redacted line, so a partial redaction cannot pass.

Suggested test additions 
     out = redact("authorization = \"Digest realm=example, nonce=deadbeef\"")
     assert "nonce=deadbeef" not in out
+    assert "realm=example" not in out
     assert "Digest" not in out
...
     out = redact("Error 401: Authorization: Bearer abc123 rejected")
     assert "abc123" not in out
+    assert "rejected" not in out
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_redact_scrubs_arbitrary_auth_scheme():
out = redact("authorization = \"Digest realm=example, nonce=deadbeef\"")
assert "nonce=deadbeef" not in out
assert "Digest" not in out
def test_redact_preserves_text_before_header_and_other_lines():
out = redact("Error 401: Authorization: Bearer abc123 rejected")
assert "abc123" not in out
assert "Error 401:" in out
multiline = redact("GET /users HTTP/1.1\nAuthorization: Bearer abc123\nAccept: */*")
assert "abc123" not in multiline
assert "GET /users HTTP/1.1" in multiline
assert "Accept: */*" in multiline
def test_redact_scrubs_arbitrary_auth_scheme():
out = redact("authorization = \"Digest realm=example, nonce=deadbeef\"")
assert "nonce=deadbeef" not in out
assert "realm=example" not in out
assert "Digest" not in out
def test_redact_preserves_text_before_header_and_other_lines():
out = redact("Error 401: Authorization: Bearer abc123 rejected")
assert "abc123" not in out
assert "rejected" not in out
assert "Error 401:" in out
multiline = redact("GET /users HTTP/1.1\nAuthorization: Bearer abc123\nAccept: */*")
assert "abc123" not in multiline
assert "GET /users HTTP/1.1" in multiline
assert "Accept: */*" in multiline
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_api.py` around lines 77 - 90, Strengthen
test_redact_scrubs_arbitrary_auth_scheme and
test_redact_preserves_text_before_header_and_other_lines to verify complete
authorization-value removal: assert realm=example is absent from the Digest
output and trailing text such as rejected is absent from the single-line output,
or assert the exact expected redacted lines.



# ───────────────────────────── user fetch ───────────────────────────────────
@responses.activate
def test_fetch_user_ok(user_payload):
Expand Down
7 changes: 7 additions & 0 deletions tests/test_sanitize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ def test_sanitize_strips_every_control_character():
assert sanitize_text(CONTROL_CHARS) == ""


def test_sanitize_strips_bidi_format_controls():
"""Cf bidi marks outside the common LRE/RLE set must also be dropped."""
assert sanitize_text("evil\u061ctext") == "eviltext" # Arabic Letter Mark
assert sanitize_text("a\u08e2b") == "ab" # Arabic Disputed End of Ayah
assert "\u061c" not in sanitize_text("x" * 50 + "\u061c" * 50)


def test_sanitize_preserves_normal_text():
assert sanitize_text("Hello, World! 123 — ok") == "Hello, World! 123 — ok"

Expand Down
Loading