Skip to content

fix(security): close credential persistence gaps - #232

Open
saagpatel wants to merge 3 commits into
mainfrom
codex/githubrepoauditor-security-local-20260821
Open

fix(security): close credential persistence gaps#232
saagpatel wants to merge 3 commits into
mainfrom
codex/githubrepoauditor-security-local-20260821

Conversation

@saagpatel

Copy link
Copy Markdown
Owner

Summary

  • reject credential aliases, provider-token shapes, URL userinfo, and sensitive query or fragment parameters before cache persistence
  • redact the same credential forms at the shared CLI output boundary
  • validate final rendered control-center Markdown before any weekly or control-center artifact write
  • add regression coverage for the demonstrated cache, CLI, and rendered-artifact bypasses

Security evidence

This follows up on current high CodeQL alerts 438, 440, 444, 445, 447, 456, and 457. Exact source-to-sink review found real shared-boundary gaps in cache persistence, CLI output, and final artifact rendering. The decision-digest and security-mode alerts project bounded aggregate values and remain unchanged.

The regression work was red-first across three passes, including URL userinfo, credential aliases, provider token families, private-key forms, URL fragments, and final rendered Markdown. An independent Luna review initially blocked publication on additional alias and fragment bypasses, then passed the corrected diff.

Verification

  • focused credential-boundary tests: 42 passed
  • aggregate/security contract controls: 232 passed in independent review
  • complete local suite: 3,548 passed, 2 skipped
  • Ruff: passed
  • CI-listed scoped mypy: 15 files passed
  • git diff --check: passed

CodeQL/provider alert disposition remains subject to fresh PR analysis; this PR does not dismiss alerts or claim provider closure before that evidence exists.

Comment thread src/github_repo_auditor/operator_control_center_artifacts.py
saagpatel and others added 2 commits August 21, 2026 18:04
…f sensitive information'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@saagpatel
saagpatel marked this pull request as ready for review August 22, 2026 01:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8673582bf6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +32 to +35
r"(?i)(?<![a-z0-9_-])(?:access[-_]?token|auth[-_]?token|refresh[-_]?token|"
r"x[-_]?api[-_]?key|api[-_]?key|apikey|authorization|client[-_]?secret|"
r"credential|github[-_]?token|password|private[-_]?key|secret|token)"
r"\s*[:=]\s*[^\s,;}&\]]+"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Distinguish credential assignments from ordinary prose

In the control-center artifact path, ordinary operator text such as OAuth refresh token: expired or Estimate token: 5 now satisfies this expression. Because write_control_center_artifacts applies the detector to the final rendered Markdown and raises before any weekly or control-center write, a non-secret queue title, summary, or recommendation containing that wording suppresses all artifacts; constrain this check to serialized credential fields or credential-shaped values and cover benign prose.

AGENTS.md reference: src/AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

r"credential|github[-_]?token|password|private[-_]?key|secret|token)"
r"\s*[:=]\s*[^\s,;}&\]]+"
)
_URL_WITH_USERINFO = re.compile(r"https?://[^/@\s]+@", re.IGNORECASE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restrict URL userinfo detection to the authority

For a valid URL whose query starts immediately after the host, such as https://example.com?email=user@example.com, this regex scans through the query and mistakes the email's @ for userinfo. The shared detector consequently rejects benign cache/artifact content, while the parallel CLI regex rewrites the URL as https://<redacted>@example.com; parse or bound the authority instead of matching until any @.

AGENTS.md reference: AGENTS.md:L65-L70

Useful? React with 👍 / 👎.

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