Skip to content

feat(docs): add arch doc for isolated jobs execution pattern - #438

Draft
florkbr wants to merge 1 commit into
OpenShift-Fleet:masterfrom
florkbr:isolated-job-execution-arch
Draft

florkbr wants to merge 1 commit into
OpenShift-Fleet:masterfrom
florkbr:isolated-job-execution-arch

Conversation

@florkbr

@florkbr florkbr commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Description

RHCLOUD-XXXXX


Blast radius


Rollback plan


Checklist

  • Tested against at least one consuming repo/service
  • No breaking changes to existing consumers (or migration path documented)
  • No hardcoded secrets, tokens, or passwords
  • Container images pinned to specific tags, not latest

AI disclosure

@Hyperkid123

Copy link
Copy Markdown
Contributor

Design review — isolated job execution

Strong direction and the core trust-boundary model is right (launcher owns Job creation, bot gets zero Job/Secret RBAC, symbolic secret allowlist, name-only template refs). Verified against the current proxy: multi-listener executor (vertex :8443, screenshot :8446, glitchtip :8447, executor gRPC :9090) and single devbot-secrets — the proposed new listener + secret split are consistent with existing patterns.

The one structural concern: the stated goal is "Claude never sees credentials," but returning raw job logs to the bot reintroduces that exposure for the job's secrets. Details below, then a recommendation to build this on OpenShell instead of a bespoke Job launcher.

Threat findings

1. Log leakage (central hole). Job mounts /secret/creds, use cases export E2E_USER/E2E_PASSWORD and oc login <token>, and the bot reads back "Job status + logs" via GET /jobs/{id}/status. A failed Playwright assertion, oc --v=8, or any env echo lands creds in logs the bot reads. Masking is best-effort (base64 / split / reordered creds evade regex). Prefer: return exit code + an allowlisted, author-guaranteed-secret-free artifact only; never raw stdout/stderr. State the residual risk explicitly.

2. Egress exfiltration. The doc says "No new NetworkPolicy — job pods use the same egress rules as bot pods." That's the wrong posture: the bot pod runs Claude; the job pod runs untrusted PR code with live creds mounted. Reusing the broad bot allowlist lets malicious code exfil via any allowlisted domain / SSO endpoint / DNS. Use a tighter per-job-type egress allowlist, distinct from the bot's.

3. REST command injection (ambiguous — nail it down). Intent reads as: bot sends {preset|job, repo, ref} by name only; command/image come from the reviewed config-repo template. But the Phase 2 YAML block mixes "what the bot sends" with "what's in the template." State the REST schema explicitly: only preset|job, repo, ref accepted; command/args/env/image/secrets from the bot payload rejected. repo/ref are bot-supplied and select the untrusted code — allowlist repos, constrain ref.

4. Config branch hijacking (Phase 2). Launcher clones the BOT_CONFIG_REPO/BOT_CONFIG_PATH that run.py supplies but never pins the branch. Force the default branch of a fixed, pre-configured repo URL; ignore bot-supplied repo/ref for config. Note the trust boundary becomes "merge access to the instance config repo" — ensure the bot can't self-merge its own template PRs.

Additional:

  • Image allowlist quay.io/redhat-services-prod/ covers every tenant in that org, not just devbot; Phase 2 lets instances set image:. Tighten to specific repo paths and digest-pin (:latest is a mutable-tag supply-chain risk).
  • Shared namespace + launcher RBAC (pods/log get, jobs delete namespace-wide): a compromised proxy pod reads all instances' job logs and deletes any job.
  • Concurrency check keys on a bot-supplied bot-instance label — spoofable; derive instance identity from the authenticated caller. The list-then-create check is also racy (TOCTOU); bound with ResourceQuota.
  • No launcher authN/authZ described on :8448. Per the doc's own networking note, any pod with the devbot part-of label reaches the proxy — so job pods (untrusted code) can also call the launcher. Require caller auth (mTLS/token) and block job pods from :8448.

Recommendation: build this on OpenShell, not a bespoke Job launcher

Given REHOR-115 (OpenShell sandbox migration) is in progress, a batch/v1 Job launcher is throwaway work. OpenShell covers most of this design and closes the worst holes natively:

This design OpenShell equivalent
job-launcher service (new) Gateway provisions sandboxes via compute driver
batch/v1 Job per task Sandbox CR (agents.x-k8s.io), one per job
secret mount + log masking Placeholder + host-side resolution — the sandbox never sees raw creds; fail-closed
per-job Squid NetworkPolicy Deny-by-default egress (host/port/binary + L7)
image allowlist Still the control point (no native exec allowlist)

The credential model is the key upgrade: creds are never plaintext inside the job pod, so verbose logs from untrusted code leak only an opaque placeholder — that resolves finding #1 rather than mitigating it, and removes the payoff of #2/#3.

Two spikes gate this:

  1. Placeholder resolution requires the proxy to see plaintext egress (protocol: rest or tls: terminate). Verify it works for Playwright SSO (placeholder typed into the login form) and oc login --token (placeholder in the Authorization header). If a use case needs the real secret in-process rather than on the wire, the placeholder model doesn't cover it — this is make-or-break.
  2. Sandbox is not batch/Job — no backoffLimit/ttlSecondsAfterFinished/activeDeadlineSeconds. Confirm it supports one-shot run-to-completion + result/artifact collection; the open questions on timeout/cleanup/retry move here.

Non-blockers: no native command allowlist (restrict via curated image + Landlock filesystem scope + fixed seccomp — same image-control approach this design already uses); external sandboxes.agents.x-k8s.io CRD dependency is already covered by REHOR-119 / REHOR-116.

Suggest folding this into REHOR-115 and gating on REHOR-119 (stage sandbox env). If E2E testing is needed before the stage sandbox is up, an interim may be warranted; otherwise wait for the migration.

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.

2 participants