Skip to content

ci/security/test: automated audit fixes for QuantPlatformKit#164

Merged
Pigbibi merged 1 commit into
mainfrom
codex/audit-fix-20260702-0107
Jul 1, 2026
Merged

ci/security/test: automated audit fixes for QuantPlatformKit#164
Pigbibi merged 1 commit into
mainfrom
codex/audit-fix-20260702-0107

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Added shared redaction for notification/log error strings so tokens, webhook query secrets, bot-token URL paths, and auth headers are not echoed.
  • Hardened Codex review gate secret detection so violations do not include matched secret values.
  • Fixed update-qpk-pin downstream dry-run verification to fail the job when any dependency check fails.
  • Removed PAT fallback from update-qpk-pin PR creation and use github.token instead.
  • Updated package license metadata to SPDX string form with setuptools>=77 to avoid upcoming setuptools build deprecation.

Problems found

  • Notification and plugin error paths could persist or print raw exception text, which may include webhook URLs, Telegram bot token paths, auth tokens, or provider response messages.
  • scripts/gate_codex_app_review.py detected hardcoded secrets but included the matched sensitive assignment snippet in CI logs.
  • update-qpk-pin.yml printed downstream install failures but still ended with success.
  • update-qpk-pin.yml preferred a PAT fallback described as bypassing a ruleset.
  • uv build emitted a future setuptools deprecation warning for project.license table metadata.

Fixes applied

  • Added quant_platform_kit.notifications._redaction.redact_sensitive_text and applied it to notification senders and strategy plugin delivery error paths.
  • Changed the Codex gate secret violation message to report only the file and generic reason.
  • Added regression tests for redaction and gate output behavior.
  • Made downstream dependency dry-run checks accumulate failures and exit 1 when any check fails.
  • Switched peter-evans/create-pull-request token to ${{ github.token }}.
  • Changed license = "MIT" and setuptools>=77.

Security impact

  • Reduces risk of leaking tokens, webhook keys, authorization headers, and secret-like assignments into logs, reports, PR checks, or alert delivery metadata.
  • No real secrets, credentials, GitHub secrets, broker credentials, or production permissions were changed.

Architecture impact

  • Adds a small shared helper in the notifications package rather than duplicating regex redaction logic across senders.
  • No public API changes intended.

Tests run

  • uv venv .venv --python 3.11
  • uv pip install --python .venv/bin/python -e . numpy pandas pytest pytest-cov ruff
  • uv pip check --python .venv/bin/python — passed
  • .venv/bin/ruff check . — passed
  • PYTHONPATH=src:. .venv/bin/python -m pytest -q tests/test_notification_redaction.py tests/test_telegram.py tests/test_strategy_plugin_telegram_notifications.py tests/test_strategy_plugin_push_notifications.py tests/test_strategy_plugin_sms_notifications.py tests/test_strategy_plugin_email_notifications.py tests/test_strategy_plugin_alert_dispatcher.py tests/test_notification_events.py — 45 passed
  • PYTHONPATH=src .venv/bin/python -m pytest -q --cov --cov-report=term --cov-report=xml tests — 444 passed, 1 skipped, coverage 60%
  • PYTHONPATH=src .venv/bin/python -m unittest discover -s tests -v — 405 passed, 1 skipped
  • actionlint — passed
  • uv build — passed
  • uvx pip-audit --path .venv — no known vulnerabilities found
  • git diff --check — passed
  • Local Codex gate diff scan — 0 violations
  • High-confidence local secret scan for private key/GitHub token/AWS key shapes — 0 findings

Failed or skipped checks with reasons

  • .venv/bin/ruff format --check . reports existing all-repo formatting drift: 155 files would be reformatted. This repository CI does not currently enforce ruff format; this PR intentionally avoids a large unrelated formatting-only diff.
  • Type checking was not run: no mypy/pyright/tsc configuration was found.
  • Live downstream install checks in update-qpk-pin were not run outside the workflow because they depend on generated constraint updates on main.

Deployment notes

  • No deployment performed.
  • This is a shared library/CI hardening change; publish/deploy should wait for required checks and human review.

Rollback plan

  • Revert commit 52e3a2c to restore previous logging, gate, workflow, and packaging metadata behavior.
  • No data migration or production state change is involved.

Manual follow-up checklist

  • Confirm GitHub Actions required checks gate and test pass on this PR.
  • Review whether repository policy should enforce ruff formatting in a separate formatting-only PR.
  • Review whether third-party GitHub Actions should be SHA-pinned across workflows.
  • If update-qpk-pin can no longer create PRs with github.token, adjust repository rules through an explicit, reviewed policy change rather than PAT bypass.

@Pigbibi Pigbibi added codex AI Codex operations automated-audit Automated audit change security Security-related change tests Test coverage or verification ci CI/CD change needs-review Needs human review and removed needs-review Needs human review labels Jul 1, 2026
@Pigbibi Pigbibi merged commit 68ee5ee into main Jul 1, 2026
2 checks passed
@Pigbibi Pigbibi deleted the codex/audit-fix-20260702-0107 branch July 1, 2026 17:24

@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: 52e3a2c89c

ℹ️ 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".

# Create a classic PAT at https://github.com/settings/tokens with `public_repo`
# scope, then add it as org secret `QSL_AUTOMATION_PAT`.
token: ${{ secrets.QSL_AUTOMATION_PAT || secrets.GITHUB_TOKEN }}
token: ${{ github.token }}

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 Use a token that can trigger PR checks

For the generated auto/qpk-pin-update PRs, ${{ github.token }} is the repository GITHUB_TOKEN; the create-pull-request action docs note that PRs created with the default token do not trigger on: pull_request or on: push workflows. I checked this repo's PR validation workflows (.github/workflows/ci.yml and .github/workflows/codex_review_gate.yml) and they are pull_request-triggered, so these automated pin PRs will be opened without the normal CI/gate checks unless someone manually retriggers them. Use a policy-approved PAT/GitHub App token or add an explicit follow-up trigger for validation.

Useful? React with 👍 / 👎.



_REDACTED = "<redacted>"
_TELEGRAM_BOT_PATH_RE = re.compile(r"(?i)(/bot)([^/\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 Redact webhook path keys before logging errors

This helper only masks Telegram-style /bot... path credentials and query-string secrets, but supported webhook providers also put credentials in URL paths (Feishu uses .../hook/KEY and ServerChan uses SENDKEY.send). When the webhook error path logs an opener/proxy exception that includes the full request URL, those provider keys remain unredacted, so the new log redaction still leaks secrets for two supported webhook formats. Add provider path patterns or parse URLs and mask sensitive path segments before logging.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-audit Automated audit change ci CI/CD change codex AI Codex operations security Security-related change tests Test coverage or verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant