Skip to content

fix: preserve Settings SSH policy and prevent gateway resize crashes - #11

Merged
meffmadd merged 7 commits into
deployfrom
settings-change
Oct 16, 2026
Merged

meffmadd merged 7 commits into
deployfrom
settings-change

Conversation

@meffmadd

@meffmadd meffmadd commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

Preserve the gateway guarantees needed to test the Settings POC with this fork image, and fix the resize/exit crash observed during manual testing.

 SSH session channel
+  accept only one successful shell/exec/subsystem start (failed starts remain retryable)
 signal request
-  consult Shell.Mode
+  consult Signal.Mode
 connection-pod readiness failure
+  roll back the newly created pod with an independent, bounded cleanup context
 agent PID before initial PTY resize
-  wait for six bytes for every PTY
+  read four bytes for connection/persistent exec; retain session PTY wait/echo framing
 execution finishes → resize queue Stop
-  close the data channel while window requests may still send
+  signal shutdown separately and idempotently; release blocked readers/writers

A late window-change previously caused panic: send on closed channel, crashed the entire gateway, broke the port-forward, and stranded connection-mode Settings pods. The resize regression exercises real execution completion and OnWindow in all three Kubernetes modes.

The other four fixes were already required by the deployed dev.box gateway. They are now ported into this PR with their existing regressions so replacing that gateway with the fork image does not reintroduce startup deadlock or weaken Settings policy/lifecycle behavior. The user explicitly approved this expanded scope.

CI publication now compares the PR head repository with github.repository, rather than testing whether the repository itself is a fork. Same-repository PRs can publish pr-11; external PRs remain excluded. Push, attestation, and summary conditions match. The earlier successful image build used push: false and did not publish its tag.

Target: TU-Wien-dataLAB/ContainerSSH → deploy, not upstream. No chart/webhook/application code, orphan GC, or automatic cluster deployment is included.

Evidence

  • Before: fork baseline reproduces concurrent/post-exit OnWindow panics and repeated-Stop panic. Each of the four added regressions also failed before its production fix: repeated starts accepted, signal policy routed through Shell.Mode, no readiness rollback, and four-byte PID blocked before initial resize.
    After: all five regressions pass with race detection:
    go test -race -mod=readonly ./internal/sshserver ./internal/security ./internal/kubernetes \
      -run '^(TestSettings|TestResizeQueueShutdown)' -count=20 -timeout=90s
    The resize regression also passed 50 race-enabled repetitions independently.
  • Native gateway build, targeted vet, and golangci-lint 2.14.0 (diff against origin/deploy) pass with zero issues.
  • Actual fork merger, security/pod validators, and native binary parse/defaults accept the existing release base plus effective Settings and ordinary-box configurations.
  • Publication conditions cover pushes, manual runs, same-repository PRs, and rejection of external PR publication. Standards/spec review found no blockers.
  • The unrelated S3 image failure is now fixed in the deploy base by upstream PR #713, which switches to pgsty/minio. Merged updated deploy into this branch and reverted the temporary source-built fixture. S3 helpers, upload tests, and their READMEs now match deploy exactly; they are not part of this PR's diff.
  • Full CI green at 5f6c644: pull-request tests and push tests pass. Lint, both snapshot builds, and image publication also pass; the push snapshot succeeded on rerun after an external release-query HTTP 403. Targeted vet/lint and all Settings/resize regressions pass locally with race detection for 20 repetitions. Local Docker integration reruns remain blocked by a full Docker VM disk.
  • New PR image published: ghcr.io/tu-wien-datalab/containerssh:pr-11@sha256:a2e40e8265787a86b5b49579dd4fc7fa7515641b244121d7e5d3e2104047914c. This image has not been rolled out or live-tested; the live evidence below refers to the earlier pinned digest.
  • User-approved live test passed (2026-10-14): dev.box release revision 21 uses ghcr.io/tu-wien-datalab/containerssh:pr-11@sha256:529a4f5618be89a56e3714fe9f77cf886a6674131e0cec5f5f1b28841d92beb9. Gateway-only upgrade preserved all other values/Secret references. Full SSH policy/lifecycle probe, 112 quit/resize packets across four fresh connections, and real OpenSSH checks passed; gateway UID remained stable with zero restarts, new Settings pods were removed after disconnect, and the ordinary box UID was unchanged. Two pre-existing crash leftovers were deliberately not deleted. Live evidence.

Merge Danger

Door: two-way

No configuration/API migration or persistent-data change. Reverting the commits restores prior behavior, including the bugs. A resize already in flight may still reach an active reader while shutdown races it; calls starting after shutdown are rejected. Readiness rollback bounds the DELETE request, not finalizer resolution or physical disappearance.

Blast Radius: Gateway

Single-start enforcement affects SSH session channels across backends; signal policy follows its own configuration. Kubernetes PID/resize behavior changes across execution modes, while new readiness rollback is connection-mode only. CI can publish trusted same-repository PR images. Live image selection and rollout are separate operations; existing orphaned Settings pods still require separately approved, UID-preconditioned cleanup.

Signal terminal-size queue shutdown separately from its data channel so late or concurrent window requests cannot panic the gateway. Release blocked operations, reject future resizes, and make Stop idempotent without locking across a send.

Add regression coverage through real execution completion and OnWindow in connection, persistent, and session modes. Reproduced the original send-on-closed-channel panic; 50 race-enabled repetitions, vet, and gateway build pass.

Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
Return the capture helper error last and explain the internal-package test annotation, following existing regression-test conventions.

Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
Port regression-tested single-start, signal-policy, connection readiness rollback, and mode-aware PID framing fixes from dev.box. Keep the resize shutdown fix and validate all cases with the race detector.

Publish same-repository PR images even when this repository is itself a fork; continue excluding external PR publication, with matching attestation and summary conditions.

Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
@meffmadd meffmadd changed the title fix(kubernetes): prevent terminal resize panics during shutdown fix: preserve Settings SSH policy and prevent gateway resize crashes Oct 14, 2026
Both PR and push CI failed TestSmallUpload, TestLargeUpload, and TestS3 because the public minio/minio image is no longer available. Official Quay images and binary downloads are also unavailable.

Build the official RELEASE.2025-10-15T17-29-55Z source commit as a local Docker fixture and reuse the existing S3 helper for upload tests. Keep integration assertions enabled, use dynamically allocated ports, and clean up temporary storage via testing.T.

Validation: all three original failures reproduced locally; pinned source builds; targeted vet and golangci-lint pass; Settings and resize regressions pass with race detection (20 repetitions). Local Docker integration verification is blocked by a full Docker VM disk; validate the complete suite in CI.
CI built the pinned source successfully, but Docker returned no port mapping because the replacement image lacked the original image's EXPOSE metadata. Declare port 9000 so the existing test helper can allocate and discover its host port.
Revert the source-built MinIO fixture and upload-helper refactor. The updated deploy branch already carries upstream's minimal switch to pgsty/minio, so Settings does not need a separate test-infrastructure implementation.
@meffmadd
meffmadd merged commit 546c703 into deploy Oct 16, 2026
13 of 14 checks passed
meffmadd added a commit that referenced this pull request Oct 16, 2026
Bring in PR #11 (Settings SSH policy and gateway resize crash fixes) so
the error-messages branch stays current with deploy: session signals now
honor the dedicated signal policy, session channel handlers start only
once, and the kubernetes resize shutdown and PID/rollback regressions are
covered. No interaction with the authorization-denial banner work.

Signed-off-by: Matt, Matthias <matthias.matt@tuwien.ac.at>
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