Skip to content

fix(security): keep gateway matrix metadata opaque#95

Merged
Pigbibi merged 3 commits into
mainfrom
codex/harden-diagnose-matrix-20260713
Jul 12, 2026
Merged

fix(security): keep gateway matrix metadata opaque#95
Pigbibi merged 3 commits into
mainfrom
codex/harden-diagnose-matrix-20260713

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • pass only opaque target indexes through workflow matrices so job names do not expose gateway metadata
  • resolve and mask target configuration inside each privileged job before authentication
  • add read-only runtime architecture and entrypoint metadata to Gateway diagnostics

Validation

  • bash tests/test_install_2fa_bot_watcher.sh
  • bash tests/test_wait_for_ib_gateway_ready.sh
  • bash tests/test_workflow_shared_config.sh
  • bash tests/test_docker_compose_ports.sh
  • bash tests/test_gateway_recovery_scripts.sh
  • bash tests/test_run_override_api_config.sh
  • actionlint .github/workflows/*.yml
  • git diff --check

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi Pigbibi enabled auto-merge (squash) July 12, 2026 21:28
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

Merge allowed: blocking findings were cleared by independent Codex arbitration

⚖️ Codex Review Arbitration

clear: The prior blocking issues are demonstrably fixed in the cumulative diff. For .github/workflows/capture-screen.yml, .github/workflows/diagnose.yml, and .github/workflows/remote-maintenance.yml, the authentication fields are no longer masked before being exposed as step outputs: the metadata step now calls core.setOutput(...) for gcp_project_id, gcp_workload_identity_provider, and gcp_workload_identity_service_account while explicitly excluding those keys from the earlier core.setSecret(...) call set, and a separate later step masks those values. That removes the exact failure mode from the prior reliability finding where masked outputs could become blank. For .github/workflows/remote-maintenance.yml, the resolver now emits {target_index, target_digest} and the privileged job recomputes sha256(JSON.stringify(target)) after reparsing IB_GATEWAY_TARGETS_JSON; if the reparsed entry at that index no longer matches the originally resolved target, the job fails with Resolved gateway target changed between jobs instead of running against a different gateway. That satisfies the prior logic requirement to bind the privileged job to a stable identifier and verify it before use. No contract conflict is present.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Security in .github/workflows/capture-screen.yml

The new matrix still publishes a deterministic SHA-256 of each full target config via target_digest. Anyone who can read the workflow graph can use that digest as a stable identifier and, because the target schema is small and predictable (project IDs, zones, instance names, secret names, etc.), brute-force likely configs offline to recover which gateway target was selected. That undermines the goal of keeping matrix metadata opaque, and the same pattern is repeated in diagnose.yml and remote-maintenance.yml. (line 73)

Suggestion: Do not expose an unhashed or unkeyed fingerprint of the target config in the matrix. Pass only an opaque per-run nonce/HMAC generated with a repository secret, or persist the resolved target mapping in workflow-scoped state that downstream jobs can read without publishing a stable verifier.

2. 🟡 [MEDIUM] Reliability in .github/workflows/capture-screen.yml

Each privileged job reparses the current IB_GATEWAY_TARGETS_JSON and hard-fails if sha256(JSON.stringify(target)) no longer matches the resolver output. Because JSON.stringify is sensitive to object key order, a semantically equivalent rewrite of the repo variable, or any benign edit between the original run and a rerun of a failed job, will invalidate the digest and block capture/diagnose/maintenance even though the selected target is still valid. (line 112)

Suggestion: Avoid binding downstream execution to a hash of mutable repo-variable text. Either canonicalize the object before hashing and compare only immutable fields, or resolve the target by a stable identifier/name at job runtime and store the originally resolved config in workflow-scoped state instead of reparsing live repo variables.


Review by Codex PR Review bot • PR

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

@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: 1a95f18300

ℹ️ 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 .github/workflows/capture-screen.yml Outdated
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi Pigbibi merged commit 6cc778e into main Jul 12, 2026
3 checks passed
@Pigbibi Pigbibi deleted the codex/harden-diagnose-matrix-20260713 branch July 12, 2026 21:45
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