Skip to content

Add config-only IBKR Scheduler deadline sync - #346

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/ibkr-run-deadline-config-only
Closed

Add config-only IBKR Scheduler deadline sync#346
Pigbibi wants to merge 2 commits into
mainfrom
codex/ibkr-run-deadline-config-only

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add a manual, main-only config path for the four allowlisted gateway /run Scheduler jobs
  • preflight immutable job name, enabled state, HTTP method, and endpoint path before any update
  • update only attemptDeadline=330s, then read back all four run jobs and four /health warmups
  • fail closed if a warmup differs from GET /health with a 60s deadline

The workflow contains no Cloud Run operation, Scheduler job trigger, create path, /run, or /dry-run request.

Validation

  • focused scheduler/runtime tests: 81 passed
  • actionlint
  • workflow YAML parse
  • focused ruff
  • compileall
  • forbidden-operation scan
  • git diff --check

Full local suite remains baseline-noisy: current branch 11 failures / 317 passed / 5 skipped; detached origin/main baseline 14 failures / 301 passed / 5 skipped. No failure is in the changed config-only files or focused tests.

Deployment

Not applied. This PR only creates a reviewed config-only path; it does not trigger a workflow or Scheduler job.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: scripts/sync_run_scheduler_deadlines.py still accepts any HTTPS *.run.app hostname whose host merely startswith(f"{expected_service}-") and ends with .run.app, while only separately checking the path and OIDC values. That does not prove the Scheduler target is the exact allowlisted Cloud Run service: a different service whose name begins with the allowlisted service name plus an extra suffix would satisfy the prefix test, and the code would also accept matching oidcToken.audience for that repointed host. The current source therefore does not satisfy the config-only destination contract the script claims to enforce, so the blocking finding remains valid. This is consistent with, not contradictory to, the prior blocking finding’s requirement to validate the full destination contract.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Security in scripts/sync_run_scheduler_deadlines.py

The host allowlist check only requires the Cloud Run hostname to start with "<expected_service>-" and end with .run.app. A repointed job targeting a different service such as https://<expected_service>-evil-xyz.a.run.app/run will still pass preflight, because that hostname shares the same prefix. That means this "config-only" sync can silently bless and preserve a compromised Scheduler target instead of failing closed, sending production /run traffic to the wrong service. (line 88)

Suggestion: Tighten the destination check so it proves the service name exactly matches the allowlisted service, not just a prefix. For example, parse the first DNS label and require it to match the Cloud Run default-domain shape for that exact service, or compare against a separately stored exact service URL/resource identity instead of startswith(...).


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

PERMANENT_FREEZE / HUMAN_REQUIRED

Stop rule reached: the same blocking fingerprint 2ee8853026561abb67d0 repeated after the single concentrated closure. No further changes, reruns, bypass, merge, workflow dispatch, or production apply will be performed on this PR.

The remaining finding is valid: a hostname prefix/suffix check cannot prove the target is the exact allowlisted Cloud Run service. A future fresh reslice must resolve each fixed service through a read-only service describe (or an independently reviewed exact URL allowlist), then require exact URI origin, exact OIDC audience, and exact scheduler identity before changing only attemptDeadline.

Live deadlines are already four run=330s and four warmup=60s from the earlier natural post-merge deployment; this PR was never dispatched.

@Pigbibi Pigbibi closed this Jul 15, 2026
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.

1 participant