Skip to content

ci/security/test: automated audit fixes for QuantRuntimeSettings#138

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

ci/security/test: automated audit fixes for QuantRuntimeSettings#138
Pigbibi merged 1 commit into
mainfrom
codex/audit-fix-20260702-0147

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Automated audit fixes for QuantRuntimeSettings focused on runtime-switch configuration consistency, generated asset idempotency, Python quality checks, and deployment workflow hardening.

Problems found

  • platform-config.json referenced default strategy profiles that were missing from the generated strategy catalog (binance and QMT conservative ETF profile path).
  • build_runtime_switch.py treated binance as a supported platform via shared runtime settings, but lacked default scope/service/dry-run variable mappings, so Binance manual switch builds could fail.
  • manual-strategy-switch.yml did not expose binance in the workflow input choices and missed QMT/Binance repo override variables.
  • inject_platform_config.py was not idempotent: repeated generation accumulated duplicate Generated by inject_platform_config.py comments in index.html/page_asset.js.
  • Deploy workflow allowed Worker asset validation to fail open via continue-on-error: true.
  • Deploy workflow used floating wrangler@latest.
  • ruff check python reported existing Python style/unused-variable issues.

Fixes applied

  • Added Binance runtime switch defaults: repository scope, BINANCE_DRY_RUN, and empty service-name default.
  • Added binance to manual switch workflow choices and added QMT/Binance repository override env vars.
  • Made QMT/Binance platform sync dispatch skip explicitly when no Cloud Run sync workflow is configured.
  • Replaced Binance default strategy with existing crypto_equity_combo.
  • Added missing cn_industry_etf_rotation profile to platform-config.json and regenerated strategy assets.
  • Made inject_platform_config.py replace the whole generated block idempotently.
  • Removed deploy validation continue-on-error and pinned Wrangler to 4.106.0.
  • Added regression tests for platform/workflow consistency, default profile existence, Binance switch defaults, and generated block count.
  • Formatted Python scripts/tests with ruff format and removed the unused variable flagged by ruff.

Security impact

  • No secrets, tokens, broker credentials, or production permissions were added or changed.
  • Deployment now fails closed when Worker asset validation fails.
  • Wrangler deploy tool version is pinned instead of floating on latest.
  • Local high-confidence secret scan found no secrets in the working tree/diff.

Architecture impact

  • Keeps the existing config-driven architecture.
  • Strengthens the single-source-of-truth contract between platform-config.json, generated assets, and manual switch workflow choices.
  • No public API or schema-breaking change intended.

Tests run

  • uv venv .venv --python 3.12
  • uv pip check --python .venv/bin/python
  • .venv/bin/python python/scripts/build_config.py --check
  • .venv/bin/python python/scripts/runtime_settings.py validate
  • .venv/bin/python -m unittest discover -s python/tests -v — 58 tests passed
  • .venv/bin/python python/scripts/check_internal_dependency_matrix.py --projects-root .. --json --strict --require-consumer-files
  • /usr/local/opt/node@22/bin/node --experimental-default-type=module tests/strategy_switch_worker_validation.mjs
  • /usr/local/opt/node@22/bin/node --test tests/*.js tests/*.mjs — 2 test files passed; JS test script reports 125 internal assertions passed
  • uvx ruff check python
  • uvx ruff format --check python
  • actionlint
  • uvx pip-audit --path .venv
  • uv build python
  • git diff --check
  • local high-confidence secret scan — TOTAL=0
  • Generated asset idempotency check: repeated build_platform_config.py + inject_platform_config.py + sync_strategy_switch_page_asset.py kept the generated diff hash unchanged

Failed or skipped checks with reasons

  • bash python/shell/checkout_internal_dependency_consumers.sh --output-root .. was not completed locally because macOS /bin/bash is 3.2 and lacks mapfile; CI Ubuntu bash should cover this path. The Python matrix check was run directly against local consumer checkouts and passed.
  • npm audit was not applicable: this repository has no package.json or Node lockfile.
  • Node emitted MODULE_TYPELESS_PACKAGE_JSON during Worker module tests; this is an existing warning and did not fail tests.

Deployment notes

  • This PR does not deploy by itself.
  • After merge, the deploy workflow may run on main because Worker/config assets changed. It remains protected by the runtime-strategy-switch environment.
  • Deployment validation is now fail-closed before wrangler deploy.

Rollback plan

  • Revert the squash merge commit if workflow/config behavior regresses.
  • If a deployed Worker regresses, redeploy the previous successful main commit or revert this PR and let the protected deploy workflow run again.

Manual follow-up checklist

  • Review whether Binance should also support explicit DCA runtime controls for crypto_btc_dca; this PR only fixes the currently broken default/dispatch path.
  • Consider SHA-pinning third-party GitHub Actions in a separate supply-chain hardening PR.
  • Verify the runtime-strategy-switch environment secrets/approvals remain least-privilege before any production deploy.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

⚠️ Review skipped: The Codex review could not be completed.

No Codex service URL or API key configured. Set CODEX_AUDIT_SERVICE_URL, ANTHROPIC_API_KEY, or OPENAI_API_KEY.

Please ensure a human reviewer checks this PR before merging.

@Pigbibi Pigbibi added codex AI Codex operations automated-audit Automated audit changes security Security-related review or hardening tests Test coverage or validation changes ci Continuous integration or deployment changes needs-review Needs human review before merge and removed needs-review Needs human review before merge labels Jul 1, 2026
@Pigbibi Pigbibi merged commit acaaafd into main Jul 1, 2026
6 checks passed
@Pigbibi Pigbibi deleted the codex/audit-fix-20260702-0147 branch July 1, 2026 18:08

@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: 8252da14d0

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

- schwab
- firstrade
- qmt
- binance

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 Keep Binance DCA dispatch compatible with the builder

Adding binance here makes the console/manual workflow able to dispatch Binance switches, but the UI/backend config already advertises Binance DCA support (DCA_SUPPORTED_PLATFORMS includes binance and crypto_btc_dca has DCA defaults). For a Binance crypto_btc_dca switch with DCA controls, the workflow reaches build_runtime_switch.py, whose DCA_PROFILES still only includes the two US DCA profiles, so it exits with DCA settings are only supported for DCA strategy profiles before applying anything. Please either add the crypto DCA profile to the builder’s DCA support or don’t expose Binance until that path is handled.

Useful? React with 👍 / 👎.

Comment thread platform-config.json
Comment on lines +507 to +511
"cn_industry_etf_rotation": {
"label": "A股行业ETF轮动",
"label_en": "CN Industry ETF Rotation",
"domain": "cn_equity",
"runtime_enabled": true,

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 Mirror CN profile in frontend fallback catalog

Adding this QMT default profile to the generated catalog still leaves the checked-in browser fallback catalog incomplete: refreshStrategyProfiles() falls back to defaultStrategyProfiles in app.js whenever /api/strategy-profiles is unavailable, and that array still omits cn_industry_etf_rotation. In that API-failure/offline path the QMT default profile remains unavailable and the console falls back to another CN strategy, so please add this profile to the frontend fallback asset as well.

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 changes ci Continuous integration or deployment changes codex AI Codex operations security Security-related review or hardening tests Test coverage or validation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant