Skip to content

ci/security/test: automated audit fixes for FirstradePlatform - #183

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

ci/security/test: automated audit fixes for FirstradePlatform#183
Pigbibi merged 1 commit into
mainfrom
codex/audit-fix-20260702-0248

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Automated audit fixes for FirstradePlatform focused on runtime reliability, notification contract drift, secret-safe error handling, dependency pin consistency, and CI/Docker gates.

Problems found

  • Runtime settings could fail when optional Telegram credentials were absent and Secret Manager libraries were unavailable.
  • Optional boolean runtime flags defaulted to false when unset, including RUNTIME_TARGET_ENABLED, causing disabled-by-default behavior drift.
  • Telegram cycle summary rendering had drifted from expected localized strategy/account labels, timing lines, signal/risk lines, and execution-blocked banners.
  • Route contract tests did not include registered /health and /healthz endpoints.
  • QPK pin checker matched nearby non-QPK git SHAs and allowed failures in CI.
  • CI did not run pytest and had a duplicate pip upgrade step.
  • Docker build used a full resolver install path that conflicts with cross-repo git package metadata when QPK pin advances.
  • Runtime exception messages could echo sensitive-looking values into logs, HTTP error payloads, or Telegram runtime alerts.

Fixes applied

  • Made optional Telegram credential resolution safe when env and Secret Manager are unavailable.
  • Restored unset optional bool semantics and kept runtime_target_enabled defaulting to true.
  • Restored localized summary labels, timing/signal/risk lines, and blocked/retryable/funding banners.
  • Updated route contract tests for health endpoints.
  • Updated direct QPK pins to canonical QPK_PIN and fixed the QPK pin checker to only match QuantPlatformKit.git@<sha> refs.
  • Added CI pip check, pytest, and made QPK pin consistency a blocking check.
  • Aligned Docker git dependency install with CI by installing git packages with --no-deps.
  • Added local sensitive-text redaction for runtime errors and regression coverage.
  • Moved Ruff ignore config to [tool.ruff.lint] to remove the deprecation warning.

Security impact

  • No production secrets, broker credentials, GitHub Actions secrets, trading permissions, or cloud resources were modified.
  • Runtime error output now redacts common token/password/API-key/header/Telegram bot URL patterns before logging or alerting.
  • Local high-confidence literal secret scan found no committed secrets.
  • pip-audit found no known vulnerabilities in the installed local environment.

Architecture impact

  • No public API or strategy behavior changes intended.
  • CI and Docker now share the same safer dependency installation pattern for cross-repo git packages.
  • Redaction is local to this repo to avoid depending on private/internal APIs from a moving shared package pin.

Tests run

  • actionlint
  • .venv/bin/ruff check --exclude external .
  • .venv/bin/python -m pytest -q163 passed
  • .venv/bin/python scripts/check_qpk_pin_consistency.py
  • uv pip check --python .venv/bin/python
  • uvx pip-audit --path .venv
  • high-confidence literal secret scan (values suppressed)
  • uv build
  • docker build -t firstradeplatform-audit:local .
  • docker run --rm firstradeplatform-audit:local python - <<'PY' ... import main ... PY
  • git diff --check

Failed or skipped checks with reasons

  • No mypy/pyright config was found, so Python static type checking was not run.
  • uv build succeeds but emits a setuptools deprecation warning for project.license table syntax; left as follow-up to avoid expanding build-system requirements in this PR.
  • Full uv pip install -r requirements.txt -c constraints.txt initially exposed a cross-repo git metadata conflict after QPK pin advancement; CI/Docker already use and now consistently enforce the no-deps git install path.

Deployment notes

  • Do not deploy unless branch protection and required GitHub Actions checks (gate, test) pass.
  • This PR does not change production secrets, Cloud Run environment values, trading-account permissions, or live-trading enablement.
  • If merged to main, existing repository workflows may build/deploy according to their configured environment flags and approvals; do not bypass approval gates.

Rollback plan

  • Revert the squash merge commit from main.
  • Re-run CI and any Cloud Run runtime guard/smoke workflow normally configured for main.
  • If a deployment was triggered, redeploy the previous known-good image/revision using the existing release/deploy workflow.

Manual follow-up checklist

  • Review whether official GitHub Actions should be SHA-pinned rather than version-pinned.
  • Decide whether to update pyproject.toml license metadata and raise build-system setuptools minimum before 2027-02-18.
  • Coordinate UsEquityStrategies metadata to the canonical QPK pin to reduce reliance on --no-deps git installs.
  • Confirm branch protection required checks remain gate and test after this CI change.
  • After merge, verify any Cloud Run deploy workflow status and smoke/health checks.

@Pigbibi Pigbibi added codex AI Codex operations automated-audit Automated repository audit security Security review and hardening tests Test coverage or validation ci Continuous integration changes needs-review Requires human review auto-merge-ok Safe to auto-merge and removed needs-review Requires human review labels Jul 1, 2026
@Pigbibi
Pigbibi merged commit 91c5c45 into main Jul 1, 2026
2 checks passed
@Pigbibi
Pigbibi deleted the codex/audit-fix-20260702-0248 branch July 1, 2026 19:11

@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: 0eedf6796f

ℹ️ 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 thread main.py
)
_AUTH_HEADER_RE = re.compile(r"(?i)\b(Bearer|Basic)\s+([A-Za-z0-9._~+/=-]{8,})")
_ASSIGNMENT_RE = re.compile(
r"(?i)\b(api[_-]?key|auth[_-]?token|credential|password|private[_-]?key|secret|token)\s*[:=]\s*([\"']?)([^\"'\s,;]{8,})([\"']?)"

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 suffixed credential names

redact_sensitive_text is now used for HTTP error bodies and Telegram runtime-error messages, but this pattern only matches exact keys such as token= or password= after a word boundary. Assignment forms using this app's actual credential names, e.g. TELEGRAM_TOKEN=... or FIRSTRADE_PASSWORD=..., do not match because the sensitive word is preceded by an underscore, so an exception containing those env-style assignments would still expose the raw secret despite the new redaction path.

Useful? React with 👍 / 👎.

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

Labels

auto-merge-ok Safe to auto-merge automated-audit Automated repository audit ci Continuous integration changes codex AI Codex operations security Security review and hardening tests Test coverage or validation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant