π± Sync workflows from kubestellar/infra - #1
Conversation
|
[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 |
9a767df to
4521efb
Compare
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. π Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
- Make beads check a hard gate in CLAUDE.md (MUST run bd list/ready before any other work, added to HARD RULES as rule #1) - Set clear_on_kick: false so strategist stays at prompt between kicks instead of being destroyed and rebuilt each cycle - Add full cd paths to bd commands so they work regardless of cwd - Add error handling: stop if bd or beads dir is missing Signed-off-by: Andrew Anderson <andy@clubanderson.com>
β¦#1065) - Old install used git clone + make which fails (make not in image) - New install uses curl|bash installer from pub-sub-tmux main - The installer auto-clones to temp dir when piped (PR #1 fix) - pst-send/pst-publish/pst-subscribe now persist across pod restarts Signed-off-by: Andrew Anderson <andy@clubanderson.com>
β¦#2743) (#2772) killAgentProcesses walked /proc and SIGKILLed every process whose owner UID == the argument, with no floor guard and no self-skip. Two call sites in runCopilotDiagnostic invoked it without the agent.UID > 0 guard other sites use. agent.UID is 0 whenever UID isolation is off or the agent is missing from the UID map (LookupByName returns 0 on miss). Hive's container runs as root, so a copilot auth-failure diagnostic -> killAgentProcesses(0) SIGKILLed every root process: the shared tmux server (all sessions), hive itself, and PID 1. Observed live. The old comment claiming it was a no-op for shared dev UIDs was FALSE as root. Fix (issue #2743, candidate #1 β the critical exploitable path): - Floor guard: refuse to run for uid < minAgentUID (= baseAgentUID 2001); log a WARNING and return 0 without killing. Makes killAgentProcesses(0) a guaranteed no-op even as root. - Self-skip: never SIGKILL our own PID (mirrors reapAgentCLI). - Guard both runCopilotDiagnostic call sites with if agent.UID > 0 to make intent explicit and skip the pointless call. - Correct the misleading "no-op for shared dev UID" comment. Regression tests (kill_floor_guard_test.go) drive killAgentProcesses against a fake /proc root: sub-floor UIDs (0, 1, proxy, floor-1) kill nothing and leave a live process alone; own PID is self-skipped; a valid agent UID still sweeps its match. Addresses #2743 (candidate #1). Candidates #2-5 remain separate. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
) Follow-on PR #1 of v2/docs/design/master-key-rotation.md. Hub-internal verify-path change only β no spoke rolls, nothing fleet-visible. The session cookie is the artifact that makes a master rotation visible to users: it is the longest-lived thing bound to a generation (cookieMaxAgeDays is 7, which is exactly why defaultVerifyWindow is 7 days). Without dual acceptance, the instant an operator rotates the master every browser session on the platform dies at once. Verification now tries every generation the hub still accepts, current first; minting stays current-ONLY, because a minter that also used a previous generation would mean a rotation never converged. WHERE THE MARKER LIVES. v3 carries it as a `g` claim INSIDE the signed payload β tamper-evident, and the Node proxy (v2/proxy/server.js) parses u/iat/exp/sid and ignores fields it does not recognize, so it is additive and needs no spoke roll. A "g<N>." prefix on the cookie VALUE β the impersonation cookie's scheme β is not available here: the same value is parsed independently by the proxy, where a prefix would sit in front of the base64url body (v3) or the username (v2) and break every hosted tenant. v2 has no payload room at all, so it gets bounded trial verification instead, capped by maxLiveGenerations == 2. BACKWARD COMPATIBILITY. `g` is omitempty and every cookie in a browser today lacks it. An unmarked cookie is tried against each acceptable generation rather than rejected, so deploying this is a non-event. A marker naming a generation the hub no longer accepts falls THROUGH to trial verification: the marker can only ever make verification cheaper, never make it fail. F1 STAYS CLOSED. Every per-generation attempt passes legacySecret as "", so there is no path by which adding a generation could resurrect the deleted symmetric lane. Tested explicitly against both generations. Finiteness is unchanged and still enforced by acceptableGenerations: a previous generation past its VerifyUntil β or carrying a zero VerifyUntil, as a hand-edited file would β is excluded, so this cannot rot into the permanent compat lane F1/F2 were. Tests (all with positive controls, so "reject everything" cannot pass): TestSessionCookieSurvivesRotation TestSessionCookieAcceptedUnderCurrentGeneration TestSessionCookieRejectedAfterGenerationExpires TestSessionCookieAcceptsUnmarkedLegacyV3 TestSessionCookieAcceptsUnmarkedLegacyV2 TestSessionCookieGenerationsPreserveRevocation TestSessionCookieGenerationsDoNotWidenAcceptance TestSessionCookieStaleMarkerFallsThrough TestSessionCookieGenerationsFailClosed TestSessionCookieUnrotatedHubIsByteIdentical Signed-off-by: Andrew Anderson <andy@clubanderson.com>
β¦ alert (#7) (#3776) Follow-on PR #7 of the hub master-key rotation design (v2/docs/design/master-key-rotation.md). Hub-internal β no spoke rolls. WHAT WAS ALREADY TRUE. acceptableGenerations(now) in hub_generations.go ALREADY filters on the wall clock: it excludes any non-current generation whose VerifyUntil has passed, and treats a ZERO VerifyUntil as ALREADY EXPIRED rather than "never expires". Every verifier goes through it. So step 5's security guarantee β "after verify_until the previous generation stops being accepted, automatically, whether or not anyone is watching" β was already kept, and this PR does not duplicate, re-implement, or weaken it. If this entire lane never ran, an expired generation would still stop verifying on time. WHAT WAS MISSING, and what this adds: 1. PERSISTING THE DROP. acceptableGenerations is a pure read-path predicate; it leaves the dead entry in the set on disk forever. A generation that is no longer accepted but is still recorded is a plaintext master secret retained on the hub PVC past the point where it protects anything β the F1/F2 residue in another form. Retirement rewrites hub-generations.json without it, so the secret stops existing rather than merely stopping being honoured. 2. THE PINNED-OPEN ALERT. Unconditional wall-clock retirement can and will strand unconverged spokes. That is correct, but it must not be silent. THE TWO CONDITIONS ARE KEPT SEPARATE, DELIBERATELY. - RETIREMENT is a WALL-CLOCK SECURITY GUARANTEE. It fires when VerifyUntil has passed, full stop β never gated on spokes_on_previous, never on spokes_unattributed, never on whether anything was observed. Gating it on convergence would let one paused or unreachable spoke keep a superseded master live indefinitely, which is exactly the unversioned, permanent compat lane the "explicitly finite" property exists to prevent. - safe_to_retire_previous stays an OPERATOR-FACING READINESS SIGNAL meaning "retiring now costs nothing". Unchanged by this PR: it still fails closed on zero observations and on any unattributed spoke (#3766 made the latter block readiness deliberately). Retirement never reads the readiness signal. The counts feed the ALERT only β they change how loudly retirement is announced, never whether it happens. WHERE IT RUNS. retireExpiredGenerationsIfDue() on the existing SHA poller in saas.go, alongside reconcileNetAdminIfDue / reconcilePerHiveEnvIfDue, throttled to 15m β no new scheduler. The throttle bounds how long a dead entry LINGERS ON DISK, never how long it is ACCEPTED. PERSIST BEFORE INSTALL, matching rotateMasterSecret: a failed write leaves the in-memory set untouched and retries next sweep, so a failed retirement is a clean no-op rather than a half-applied state that reappears at the next hub roll. lastKeyRotation is carried through unchanged β retirement is not a rotation and must not reset the double-rotation cooldown. Alert severity: warn when the window closes within 24h with spokes still on the generation (sized to exceed a full ~5.5h reconcile sweep, so it is actionable rather than a post-mortem), stranded once it has closed. spokes_unattributed counts as "still carrying the old key" β broken now, not lagging. IDs, counts and timestamps only; never key material. TESTS TestRetireExpiredGenerationsIsPositiveControl β fails in BOTH directions (never retires / retires unconditionally / retires current). TestRetireTreatsZeroVerifyUntilAsExpired β the invariant, on the new path. TestRetireHappensEvenWithSpokesStillOnGeneration β the ANTI-PIN test: the whole fleet is on the old generation, retirement fires anyway and the alert reads stranded. TestGenerationPinAlertSeverities β alert policy across the timeline, including unattributed-only and zero-observation cases. TestSafeToRetirePreviousStillFailsClosed β readiness unchanged. TestRetirementSurvivesRestart β persist, reload, still retired; 0600 held. TestFailedPersistLeavesRetirementUnapplied β failed write is a no-op. TestMergedReadersRejectRetiredGeneration β all four merged readers (#1 cookie, #2 bearer, #3 per-hive env, #4 rotate) REJECT rather than error against a just-retired generation. TestSweepGenerationRetirementIsIdempotent β safe on every poller tick. Regression replay: gating retirement on spokes_on_previous > 0 still compiles and fails TestRetireHappensEvenWithSpokesStillOnGeneration with "retired [], want [1]"; restored, all nine pass. Full ./pkg/hub/ suite green. The #2 (2234750) trap is avoided: the new test helper goes through setHubSecret rather than assigning hubSecret directly, so the new generation-set reader is never tested against a discarded secret. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
β¦#6) (#3778) Follow-on PR #6 of the master-key rotation design. Unlike #1-#5, the verifier here is the SPOKE, not the hub: the SSO handoff token and the hub session cookie are minted on the hub and verified by Go in pkg/dashboard and, independently, by Node in v2/proxy/server.js. Those verifiers hold one public key and no generation concept. So the failure mode is the mirror of the earlier ones. The instant a hub rotates it mints under generation N while ~65 spokes still hold N-1's public key, and they cannot verify anything the hub now mints β for the ~6h the rate-limited reconcile lane takes to walk the fleet, not for the 30 minutes an impersonation cookie lives. The fix is plurality on the spoke: hold both live generations' public keys and try each, bounded by maxLiveGenerations == 2. ENV CONTRACT: a second var (HIVE_SSO_PUBLIC_KEY_PREV, HIVE_SESSION_PUBLIC_KEY_PREV), not a delimited list in the existing var. A list was measured, not assumed, and rejected: Node's Buffer.from reads "<hex>,<hex>" as 32 bytes (silently truncating to the first key) while Go's hex.DecodeString errors and fails closed. The two languages disagree about exactly "does hosted SSO still work on an un-rolled spoke", so a list would kill SSO on every spoke the reconcile lane had patched but that had not yet rolled the new image. An extra var has a defined meaning to old readers: they do not read it, so they cannot misread it. It is also the only encoding whose key set can SHRINK cleanly at retirement. ORDERING: this ships the VERIFIER plurality and the provisioning of the second var, and cannot itself cause a rotation β that is #4's admin endpoint. Verifiers therefore learn to accept two keys strictly before the hub can ever hand out a second one. NO-OP WITH ONE GENERATION, which is every hub today: previousPublicKeys returns empty, desiredPerHiveEnv emits no _PREV var, perHiveEnvDrift sees no drift, no spoke is patched and no pod rolls. Asserted by name, not by count, in TestDesiredPerHiveEnvPrevVarsFollowGenerations. The _PREV vars are self-clearing: once VerifyUntil passes, acceptableGenerations drops the generation, the var stops being wanted, and perHiveEnvDrift now reports the reverse direction (present when it should be absent) so the patch REMOVES it. Without that the retired key would sit on all 65 spokes forever β the unversioned permanent compat lane the design exists to prevent, as a stale env var. Invariants held: private seeds never leave the hub (asserted mechanically in TestPrivateSeedsNeverLeaveTheHub); malformed second key cannot disable the first (dropped by strict validation in both languages, which also avoids ed25519.Verify's panic on a wrong-length key); VerifyUntil.IsZero() == already expired; no symmetric fallback is reintroduced (F1 stays deleted β the legacy lane is tried once, outside the key loop, and is not multiplied across generations); IS_HOSTED is byte-identical, deliberately keyed off the primary key so a malformed var cannot silently flip a spoke to self-hosted. The terminal key path in server.js is untouched (PR #5's scope). Signed-off-by: Andrew Anderson <andy@clubanderson.com>
β¦ detection What changed: - Stage src/deploy/legacy-redirect/: a Deployment+Service backend that redirects hive.kubestellar.io -> hive.hivecommons.dev (301) and dibs.kubestellar.io -> dibs.hivecommons.dev (308, inert until #5925), preserving $request_uri. Needed because the ingress-nginx permanent-redirect annotation cannot carry $request_uri (admission webhook rejects '$', snippet annotations are disabled on this cluster). - Applied to hive-oke: legacy-redirect Deployment/Service in hive-hub, repointed the live hive-hub Ingress at it, removed the lossy annotation. Verified live: hive.kubestellar.io/learn?utm_source=newsletter now 301s to hive.hivecommons.dev/learn?utm_source=newsletter (previously dropped to the bare homepage). - Updated src/deploy/dibs-domain-cutover/03- to route through the same legacy-redirect backend via a new ExternalName Service (04-) in the dibs namespace, since Ingress backends must share the Ingress's namespace. Not applied (dibs cutover is held on LE quota, #5925). - Added a drop-detection arm to bin/ga4-anomaly-detector.sh (bin/ga4_anomaly_lib.py): a domain migration collapses real traffic rather than raising an error-event count, which the existing spike-only arm could not see. Thresholds are named constants, env-overridable (GA4_ANOMALY_THRESHOLD, GA4_DROP_THRESHOLD, GA4_DROP_MIN_BASELINE_DAILY). - Confirmed item 3 (og:url/canonical) is already fixed and tested on v4; no change needed there. Why: #6430 β the hivecommons.dev migration broke GA4 in four ways; this addresses the two repo/cluster-fixable ones (#1 and #4). How tested: - bash bin/test_ga4_anomaly_detector.sh (new, 12/12 pass, hermetic) - bash bin/test_dibs_cutover_preflight.sh (42/0 -> still 29/0 pass) - bash bin/test_dibs_cutover_verify.sh (42/0 pass) - bash bin/test_bin_suites_wired.sh / src/deploy/test_deploy_suites_wired.sh - cd src && go build ./... && go test ./pkg/hub/... -run CanonicalURL (pass) - python3 src/scripts/check-docs-links.py src/docs (pass) - shellcheck on changed shell files (no new findings) - Live cluster curl verification (see PR body) Fixes #6430 Refs #5925 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andrew Anderson <andy@clubanderson.com>
This PR syncs the caller workflows from
kubestellar/infra.These workflows call reusable workflows from
kubestellar/infra:Standard Workflows:
add-help-wanted.yml- Add help-wanted label to issuesassignment-helper.yml- Handle issue assignmentsfeedback.yml- Collect feedbackgreetings.yml- Welcome new contributorslabel-helper.yml- Manage labelspr-verifier.yml- Verify PR contentspr-verify-title.yml- Verify PR title formatscorecard.yml- Security scorecardstale.yml- Mark stale issues/PRsAgentic Workflows (Copilot Integration):
ai-fix.yml- Assign Copilot to issues withai-fix-requestedlabelcopilot-automation.yml- Automate Copilot PR processing (DCO, labels)copilot-dco.yml- Override DCO for Copilot PRsAuto-generated by workflow sync