Skip to content

ci/security/test: automated audit fixes for FirstradePlatform#186

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

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

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • harden the Codex review gate so secret-like findings are redacted instead of echoing matched values
  • add a regression test for the redaction behavior
  • extend CI/local verification docs with package build coverage
  • remove the setuptools project.license table deprecation warning by switching to SPDX string form

Problems found

  • the audit gate reported matched secret-like fragments directly in violation messages
  • CI did not build the package artifact, so packaging regressions could slip through
  • local contribution guidance did not document pip check, Ruff, or build verification
  • pyproject.toml used deprecated table syntax for project.license

Fixes applied

  • redact secret values in scripts/gate_codex_app_review.py and keep only normalized field names
  • add tests/test_gate_codex_app_review.py regression coverage
  • add python -m build to .github/workflows/ci.yml
  • document pip check, Ruff, pytest, and build in CONTRIBUTING.md
  • update pyproject.toml license metadata to "MIT"

Security impact

  • reduces risk of secret-like content being echoed in CI logs or PR gate output
  • no production credentials, runtime secrets, broker permissions, or deployment settings were changed

Architecture impact

  • low risk; no public API, execution path, or runtime contract changes
  • build validation is now part of CI for earlier packaging failure detection

Tests run

  • .venv-ci/bin/python -m pip check
  • .venv-ci/bin/ruff check --exclude external .
  • .venv-ci/bin/python -m pytest -q tests/test_gate_codex_app_review.py
  • .venv-ci/bin/python -m pytest -q
  • SSL_CERT_FILE=$(.venv-ci/bin/python -c 'import certifi; print(certifi.where())') .venv-ci/bin/python scripts/check_qpk_pin_consistency.py
  • .venv-ci/bin/python -m build
  • actionlint
  • git diff --check

Failed or skipped checks with reasons

  • pip-audit was not run because pip_audit is not installed in the local validation environment
  • repository-wide secret-pattern grep produced expected hits in docs/tests/workflows, but no real secret values were exposed in this diff

Deployment notes

  • no deployment triggered locally
  • if merged, normal repository CI should validate the same paths in GitHub Actions

Rollback plan

  • revert commit 105596b or revert the PR squash commit if follow-up issues appear

Manual follow-up checklist

  • review PR diff for gate wording and CI scope
  • confirm required GitHub Actions checks pass on the PR
  • merge only after human review if branch protection requires it

@Pigbibi Pigbibi added codex AI Codex operations github_actions Pull requests that update GitHub Actions python Pull requests that update python code human-review-required Requires human review before merge labels Jul 1, 2026
@Pigbibi Pigbibi merged commit bf69113 into main Jul 1, 2026
2 checks passed
@Pigbibi Pigbibi deleted the codex/audit-fix-20260702-0534 branch July 1, 2026 21:41

@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: 105596bfec

ℹ️ 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 pyproject.toml
readme = "README.md"
requires-python = ">=3.11"
license = { text = "MIT" }
license = "MIT"

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 Require setuptools 77 for SPDX license metadata

With this change, builds now depend on PEP 639/SPDX-string project.license support, but build-system.requires still allows setuptools>=68. In any build environment or package mirror that resolves setuptools 68–76, the backend can reject this field as requiring the old dict/table form; the PyPA guide lists setuptools 77.0.3 as the version that introduced this support (https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license). Please either raise the build-system lower bound or keep the table syntax until the lower bound is updated.

Useful? React with 👍 / 👎.

if m:
violations.append(f"**Hardcoded secret** in `{current}`: `{m.group(0)[:100]}`")
field = re.sub(r"\s+", "_", m.group("field").strip().lower())
violations.append(f"**Hardcoded secret** in `{current}`: `{field}=<redacted>`")

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 Preserve distinct secret hits when redacting values

When a PR adds multiple hardcoded secrets with the same normalized field in the same file, such as two *_API_KEY assignments, this redacted message is identical for each match and the existing dict.fromkeys de-duplication collapses them into one violation. The gate still blocks, but it underreports the number of secrets and can force repeated CI cycles as each hidden occurrence is discovered only after earlier ones are fixed; include a non-secret discriminator such as an occurrence or line number.

Useful? React with 👍 / 👎.

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

Labels

codex AI Codex operations github_actions Pull requests that update GitHub Actions human-review-required Requires human review before merge python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant