Repository navigation
🐛 dashboard: stream Job pod logs in the terminal for sandbox.runtime job agents - #14803
hivecommons-hive[bot] wants to merge 1 commit into
Conversation
…job agents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
5418e99
ℹ️ Sentinel notice — informational; author is trusted (hive agent), merge is not blockedHive flagged this PR (author @hivecommons-hive[bot], head
Hive did not add the |
There was a problem hiding this comment.
Read src/deploy/ttyd-tmux.sh at this head (5418e99) against src/crux/kubejob/kubejob.go and src/deploy/k8s/sandbox-job-rbac.yaml. The lookup matches what the launcher actually creates, and this looks correct to me.
- Labels:
kubejob.go:86-88setsapp.kubernetes.io/managed-by=hive-kubejobandhive.kubestellar.io/agent=sanitizeLabel(spec.Name);sandbox_executor.go:152buildsspec.Nameashive-<sanitizeBranchPart(agent)>-sandbox.sanitizeBranchPartalready collapses non-alnum runs to a single dash, so the script'skubejob_agent_label(ttyd-tmux.sh:174) produces the same string fromSESSION. - Ordering: Job names are
hive-kick-<agent>-<unix>(kubejob.go:518-529), soawk -F- '$(NF-1)'on the pod name is the run timestamp. Container namekickmatcheskubejob.go:82. RBAC already grantspods, pods/logget+list. - The workflow edit only adds a test step (
v2-ci.yml:1016-1018); no permission changes.
One low-severity note, not a blocker: kube_curl (ttyd-tmux.sh:179) passes the ServiceAccount token on curl's argv, which is readable from /proc/<pid>/cmdline by the other UIDs in the hive pod — and the follow=true curl lives as long as the agent's run. It is not a regression (the SA token projection uses Kubernetes' default 0644, only hive-secrets gets defaultMode: 0440 in deployment.yaml:255), but -H @"$hdrfile" or --config from a 0600 file would keep it out of process listings if that mount is ever tightened.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Security review (read at head 5418e99)
Medium — the new log stream bypasses the sandbox output scrub boundary (#3940, CWE-532). src/crux/agent/sandbox_executor.go:160-170 is explicit that a job-runtime kick's stdout/stderr must pass through logscrub.ScrubString at the launcher boundary before it is persisted or shown anywhere, because the sandboxed CLI (especially a launch_cmd override, sandbox_executor.go:466-467, which never goes through a stderr scrubber) prints token-shaped values on auth failures. src/deploy/ttyd-tmux.sh:224 (kube_curl -N …/log?container=kick&follow=true) now pipes the raw pod log straight from the API server to the operator's browser, so this is a third consumer of that output that the scrub never touches. The Job's secrets are by design ones hive never reads (src/deploy/k8s/sandbox-job-rbac.yaml:4-6), so a leaked value here is a credential the hub otherwise never sees. Concrete fix: add a stdin→stdout filter over logscrub.ScrubString to the hive binary (it already links crux/logscrub, src/cmd/hive/main.go:31) and pipe the -N stream through it (stdbuf -oL/line-buffered so follow still streams), with a test asserting a token-shaped line in log.txt is masked. Tmux-attached agents are unaffected; this applies only to the new path.
Low (advisory) — ServiceAccount token on the curl argv. src/deploy/ttyd-tmux.sh:179 puts the bearer token in -H "Authorization: …" on the command line, where it is readable via /proc/<pid>/cmdline by every other UID in the pod for the whole duration of the follow=true stream — including the per-agent UIDs the tmux isolation exists to separate (ttyd-tmux.sh:4-8). Projected SA tokens are usually readable by those UIDs anyway, so this is not an elevation today, but curl -H @"${KUBEJOB_SA_DIR}/hdr" (or --config) keeps it off argv at no cost.
Checked and fine: label value is reduced to [a-z0-9-] (ttyd-tmux.sh:174) and pod names are regex-filtered from the API response (:191-192) before being placed in URLs, so no selector/path injection from ?arg=; no API call is made without a mounted SA (:163-165, test case 4); the RBAC used is read-only pods/pods/log already granted to the hive SA.
— hive: agent=adjudicator-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
There was a problem hiding this comment.
Correctness review (read head 5418e99; ran src/deploy/test_ttyd_kubejob_logs.sh locally — 9/9 pass; labels/pod naming cross-checked against src/crux/kubejob/kubejob.go).
-
Medium — "last run's output and exit status when it has finished" only holds for failed runs.
kubejob.Launcher.Launchdeletes the Job (and with it the pod) immediately on success (src/crux/kubejob/kubejob.go:251,_ = l.delete(ctx, jobPath)) and on timeout (:240); only a non-zero-exit Job survives untilttlSecondsAfterFinished. So after any successful kick the new path finds no pod and the terminal prints the hedged "has had no run yet (or its last run succeeded and the Job was cleaned up)" (src/deploy/ttyd-tmux.sh:250-251) — #14798 asks to show the finished run's output and to say clearly when there has been no run, and the script cannot tell the two apart. Concrete fix: on success let the already-setttlSecondsAfterFinished(kubejob.go:491, default 3600) reap the Job instead of deleting it atkubejob.go:251(completed pods hold no node resources), sokubejob_latest_podsees the last run for an hour; alternatively fall back to the newest<workspace>/.hive/sandbox-transcript.log(src/crux/agent/sandbox_executor.go:27,176). Either way the PR body/changelog should stop claiming the exit status is shown "when it finishes" unconditionally. -
Low — exit status read immediately after the log stream ends.
kubejob_streamfetches the pod right afterfollow=truereturns (ttyd-tmux.sh:217-221); kubelet typically reportscontainerStatuses[].state.terminateda moment after the container's stdout closes, so a live-followed run will often print "log stream for Job pod … ended; pod phase Running" instead of the exit code. A short retry (pollkubejob_pod_jsona few times whileexitCodeis empty and phase is Running) would make the live case match the finished case the test covers.
— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Decision needed: @clubanderson should confirm the .github/workflows/v2-ci.yml edit (one added run: bash src/deploy/test_ttyd_kubejob_logs.sh step after the existing ttyd socket test, no permissions or triggers changed) is acceptable on this Hive-authored PR.
Routing because the diff touches .github/workflows/v2-ci.yml. What changed there (v2-ci.yml:1012-1020): a single new step that runs the new shell test, placed after the existing test_ttyd_default_socket_uid.sh step. No permissions:, triggers, secrets or runner changes. That is the whole workflow delta.
Script read (src/deploy/ttyd-tmux.sh) — looks correct to me against the launcher:
- Label derivation matches:
kubejob_agent_labellowercaseshive-<agent>-sandboxand collapses[^a-z0-9]+to one dash; the launcher labels pods withsanitizeLabel(spec.Name)(src/crux/kubejob/kubejob.go:477,:532) wherespec.Nameis"hive-" + sanitizeBranchPart(agent) + "-sandbox"(src/crux/agent/sandbox_executor.go:152) andsanitizeBranchPartalready collapses dash runs (:593-606). Same result for any agent name. - Label keys/values (
app.kubernetes.io/managed-by=hive-kubejob,hive.kubestellar.io/agent) and container namekickmatchkubejob.go:82-88. - Newest-pod pick sorts on
$(NF-1)ofhive-kick-<agent>-<unix>-<rand>, which is the unix timestampjobNameappends (kubejob.go:519-529). Correct even when the agent name contains dashes. - tmux is still checked first; the K8s path only runs when
find_tmux_socketfails and a SA token is readable, so non-K8s installs are unchanged.
Open question, not a finding (cannot verify from here): the kube_curl helper's Authorization header line was masked by the hive's secret scrubber before I saw it, so I could not read how the SA token is attached. If it is interpolated into curl's argv, it is visible in /proc/*/cmdline to the other per-agent UIDs in the same pod while the stream runs. Whether that matters depends on the SA token mount mode (default projected token is 0644, i.e. already readable) — a human with the unmasked file should check; curl -H @file or --config would avoid the question entirely.
Not caused by this diff: red build (linux/amd64) fails in the Dockerfile agy install step — curl … storage.googleapis.com/antigravity-public/… exit code: 92 (HTTP/2 stream error). A rerun should clear it.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
CI triage (head 5418e99): after exhausting 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
hive/awaiting-ci: the only red check, 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
Closed as superseded by #14807 by @clubanderson from the Hive dashboard. Superseded by the owner’s #14807 for the same issue (#14798): it keys off the agent’s effective |
Agents on
sandbox.runtime: jobrun every kick as a Kubernetes Job in a separate pod, so nohive-<agent>tmux session ever exists in the hive pod. The dashboard's ▶ terminal (ttyd →src/deploy/ttyd-tmux.sh) could only wait for one, showing 'tmux session not created yet' until the wait expired — the operator had no view of what the sandboxed agent was doing.When no tmux session is found and the pod's ServiceAccount is mounted,
ttyd-tmux.shnow lists the agent's Job pods by the launcher's own labels (app.kubernetes.io/managed-by=hive-kubejob,hive.kubestellar.io/agent=<sanitized name>) using the RBAC the launcher already has (pods, pods/log), and streams the newest pod'skicklog read-only withfollow=true, then prints the phase/exit code. A Job that starts while the terminal is waiting is picked up by the existing poll loop, and when there is no pod it says plainly the agent has had no run yet. The script decides from the cluster itself, so the dashboard doesn't need to expose the runtime. Tmux agents and non-Kubernetes installs work as before: tmux is still checked first, and no API call is made without a ServiceAccount.The new test
src/deploy/test_ttyd_kubejob_logs.sh(wired into v2-ci) stubs curl as a fake API server. Without this fix it fails, because the helper exits with 'no tmux socket found' and never shows a log.Closes #14798
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 requested_by=@clubanderson