From 40c7a93951c619c868128158c41c13d4fe442e53 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 13 May 2026 18:21:44 +0000 Subject: [PATCH] docs: add future work items 6-8 and clarify design doc safety gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Future Work §6: validate_host() implementation plan (CN allowlist, admin-cn flag, reject/log unauthorised OP_upgradeConfig requests) - Future Work §7: OP_replConfig access control hardening (peer validation, rate-limiting, minimum_role check) - Future Work §8: lost_and_found reserved-name validation in admin config tool, with migration path for pre-existing clusters - config-replication-design.md: expand CONF_ENABLE_UPGRADE_CONFIG safety gate section with gating prerequisites --- src/docs/FUTURE_WORK.md | 58 ++++++++++++++++++++++++++- src/docs/config-replication-design.md | 36 +++++++++++++---- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/docs/FUTURE_WORK.md b/src/docs/FUTURE_WORK.md index ba0812f6..6632f1cd 100644 --- a/src/docs/FUTURE_WORK.md +++ b/src/docs/FUTURE_WORK.md @@ -49,4 +49,60 @@ at election time rather than silently producing dual-primary split-brain. Test that a standby correctly returns an error reply with the primary's address when an admin tool sends `OP_upgradeConfig` to a standby, and the admin tool -retries against the primary. \ No newline at end of file +retries against the primary. + +--- + +## 6. validate_host() Implementation Plan + +`validate_host()` is currently a stub that returns success for all peers. Before +`CONF_ENABLE_UPGRADE_CONFIG` can be set to 1, it MUST be enhanced with a real +authorisation check. Minimum viable implementation: + +- Verify the peer's X.509 certificate Common Name (CN) against a configured + allowlist of authorised admin hosts. +- Add `--admin-cn` CLI flag or `admin_cn_list` config field to specify allowed CNs. +- Reject `OP_upgradeConfig` requests from peers whose CN is not in the allowlist + with a clear error message. +- Log all rejected attempts at WARN level with peer CN and IP. + +Future extensions (not required for gate removal): IP allowlist, role-based +access, mutual-TLS with admin-specific CA. + +--- + +## 7. OP_replConfig Access Control Hardening + +`OP_replConfig` currently serves the full cluster config blob to any peer that +completes an SSL handshake with a CA-signed cert (trusted-LAN model, §5.3). +Enhancements to consider: + +- Verify the requesting peer is a known controller (listed in `--peers`) before + serving the config blob. +- Rate-limit `OP_replConfig` requests per peer to prevent repeated disk I/O on + the primary. +- Add a `minimum_role` check (e.g., only serve to `HostRole_controller` peers, + not arbitrary SSL clients). + +--- + +## 8. Lost-and-Found Internal Keyword Validation + +The name `"lost_and_found"` is an internal controller keyword for synthetic +queue and host entries created during WAL recovery (§6.5). The controller +rejects this name at both `load_primary_config()` and `OP_upgradeConfig` entry +points. + +**Config tool responsibility**: The admin config tool (Future Work §1) MUST +validate that user-authored config files do not use the reserved names +`"lost_and_found"` for any queue or host. The tool should emit a clear error +and refuse to encode the config blob. This provides defence-in-depth: the +controller enforces at runtime, and the tool prevents at authoring time. + +**Migration path for pre-existing clusters**: If a cluster already has a +user-defined queue or host named `"lost_and_found"` before upgrading to this +controller version, the operator must rename it in the cluster config YAML +before the upgrade. The admin config tool will validate this; if bypassing +the tool and hand-crafting a Cap'n Proto blob, the controller's +`load_primary_config()` will FATAL with a message identifying the reserved +name. See `config-replication-design.md` §6.5 and §7.1. \ No newline at end of file diff --git a/src/docs/config-replication-design.md b/src/docs/config-replication-design.md index 7e74d100..a43ca7f1 100644 --- a/src/docs/config-replication-design.md +++ b/src/docs/config-replication-design.md @@ -185,13 +185,24 @@ and `validate_host()` are complete; see `FUTURE_WORK.md`): - ~170 LOC The `OP_upgradeConfig` handler is gated behind a compile-time flag -`CONF_ENABLE_UPGRADE_CONFIG` (default: 0, disabled — see §5.3 safety gate) until -two prerequisites are met: (1) `validate_host()` is implemented with a real -authorisation check, and (2) an admin tool exists to drive the endpoint (encoding -`ClusterConfig` to Cap'n Proto binary, computing SHA-256, and sending the request -via the controller's internal endpoint). Both prerequisites are tracked in -`FUTURE_WORK.md`. Phase 1 alone eliminates manual provisioning and config drift — -the core problems stated in §1. +`CONF_ENABLE_UPGRADE_CONFIG` (default: 0, disabled — see §5.3 safety gate). The +gate MUST NOT be set to 1 in any production build until BOTH prerequisites are met: + +1. **`validate_host()`** is implemented with a real authorisation check (not a stub + that returns success for all peers). Minimum viable check: verify the peer's + certificate CN against an allowlist of authorised admin hosts. See `FUTURE_WORK.md` + §6 for the implementation plan. +2. **Admin config tool** exists and has been tested end-to-end. It must encode + `ClusterConfig` to Cap'n Proto binary, compute SHA-256, connect via SSL, send + `OP_upgradeConfig`, display the reply (version tag, acked count, error), and poll + `OP_probe` after restart. See `FUTURE_WORK.md` §1 for the tool specification. + +**Prerequisite delivery commitment**: Both prerequisites are tracked in +`FUTURE_WORK.md` and MUST be completed before the next production release that sets +`CONF_ENABLE_UPGRADE_CONFIG=1`. A release may ship with the gate at 0 (Phase 1 +active, Phase 2 infrastructure compiled but inert). The gate is removed from the +codebase when both prerequisites are merged and integration-tested. Phase 1 alone +eliminates manual provisioning and config drift — the core problems stated in §1. --- @@ -1156,6 +1167,17 @@ cause unbounded growth. Operators should inspect and drain `lost_and_found` afte a config upgrade; a future `xcecli lost-and-found drain` command can automate reassignment or cancellation (§14.4 future work). +**Job submission gate**: The API server (`src/frontend/apiserver`) MUST reject job +submissions targeting the `lost_and_found` queue with a clear error: `"Cannot +submit jobs to 'lost_and_found' — this is an internal recovery queue."` The +queue appears in `xcecli queue list` for operator visibility but does not accept +external job submissions. This prevents accidental routing of production jobs to +the synthetic recovery queue and keeps `lost_and_found_job_count` meaningful +(only incremented for genuine WAL-recovered orphans, not operator mistakes). +The check is enforced at the API layer (apiserver), not in the controller, so +standby-only deployments without the API server still have the controller-side +protection of §6.5's name reservation. + ### 6.6 Downtime estimate for config upgrade The primary is unavailable from when it sets `restart_requested` until Phase 2