feat(dns): ownership-guarded managed DNS records + Cloudflare proxied mode (ADR-031) - #347
Draft
dviejokfs wants to merge 3 commits into
Draft
feat(dns): ownership-guarded managed DNS records + Cloudflare proxied mode (ADR-031)#347dviejokfs wants to merge 3 commits into
dviejokfs wants to merge 3 commits into
Conversation
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Added
- **dns:** Add ownership-guarded managed DNS records (ADR-031)
- **dns:** Harden ownership guards, add managed-record API with audit logging
### Fixed
- **dns:** Map DomainNotManaged to 404 and poison-proof keyed locks |
Foundation slice for managed DNS record automation: - OwnershipMarker: typed JSON markers in _temps-owned.<name> companion TXT records (external-dns registry pattern), versioned (v:1), strict parse so user TXT content can never read as ours - ManagedDnsRecordService: the only path for public A/AAAA/CNAME writes. Never overwrites records without a matching marker (typed RecordConflict / NotOwnedByInstance errors for the import-or-skip UI), marker-first write ordering, explicit import_record adoption flow - Cloudflare Universal SSL depth guardrail: proxied records >=2 subdomain levels are rejected at write time with a flat-hostname suggestion instead of failing at the edge with an opaque 526 - dns_instance_identity single-row table: install-scoped instance ID so two temps installs sharing a zone refuse to touch each other's records - dns_managed_domains.proxied_by_default column (default false) - ADR-031 documenting the design, alternatives, and non-goals
… logging Fixes from the ADR-031 security/code review of the foundation slice: - BLOCKING registry-TXT clobber: guarded_set/import now inspect the ownership registry name before writing; a non-marker TXT or another install's orphan marker at that name refuses with a typed conflict instead of being upserted over - BLOCKING type-scoping: registry names are per record type (_temps-owned-a.<name> etc.) and markers carry record_type; owning a name's A record no longer reads as ownership of the user's AAAA/CNAME at the same name - BLOCKING wildcard collision: injective name escaping (underscore doubling then * -> _w) so *.staging and a literal wildcard.staging can never share a registry name - TOCTOU: per-(zone,name) keyed async locks serialize guarded ops in-process (self-cleaning map, bounded memory); remaining remote window documented as accepted - marker instance field validated on parse ([A-Za-z0-9-], <=64) to keep attacker-written TXT content out of logs/UI - proxied_by_default on the managed domain is now consumed by set_managed_record; proxy gate extracted to testable check_proxy_allowed - removal granularity (whole name+type unit) documented New HTTP API (all RequireAuth + permission_check + audit logged): - GET /dns-records/ownership per-record state for the conflict UI - POST /dns-records ownership-guarded create/update - DELETE /dns-records ownership-guarded removal - POST /dns-records/import explicit adoption of an existing record - new DnsError variants mapped to 409/400 Problem responses Tests: 271 passing in temps-dns (+12), covering registry clobber refusal, foreign orphan markers, our-orphan reuse, type-scoped ownership, injective escaping, proxy gate, keyed-lock serialization and cleanup, and instance_id get-or-create incl. insert-race recovery via MockDatabase
dviejokfs
force-pushed
the
feat/adr-031-managed-dns-records
branch
from
July 14, 2026 14:57
b128813 to
361f2c1
Compare
Review follow-ups on the managed-records API: - DomainNotManaged fell through the From<DnsError> catch-all to a 500, contradicting the 404 all four /dns-records endpoints document; a domain temps doesn't manage is client input, not a server fault. Map it explicitly to 404 with an actionable detail message - KeyedLocks now recovers a poisoned map mutex instead of expect()ing; a panic there would otherwise turn every later DNS write into a panic until restart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements ADR-031: Managed DNS Records and Cloudflare Proxied Mode (
docs/adr/031-managed-dns-records-and-cloudflare-proxied-mode.md, included). Driven by #139's use case: never expose the origin IP, proxy everything through Cloudflare, and let temps create the records — without ever clobbering records temps doesn't own.The safety invariant
Temps writes into zones it does not own. Every record temps creates gets a companion TXT registry record containing a typed, versioned JSON ownership marker (external-dns registry pattern, uniform across all 6 providers).
ManagedDnsRecordServiceenforces structurally:_temps-owned-a.<name>,_temps-owned-aaaa.<name>, …) and the marker carriesrecord_type— owningappA never grants ownership of a user'sappAAAA._→__before*→_w) —*.stagingand a literalwildcard.stagingcan never share a registry name.RecordConflict, 409), another install's marker (NotOwnedByInstance, 409), or a non-marker TXT occupying the registry name itself (the marker write never upserts over foreign content, including another install's orphan marker).importis the explicit, user-confirmed adoption path; default is always never-overwrite.instanceis validated on parse ([A-Za-z0-9-], ≤64) so attacker-written TXT can't inject into logs/UI.Cloudflare proxied mode
*-staging.example.com, per feat(settings): add flat public hostname strategy #146) instead of failing at the edge with an opaque 526.dns_managed_domains.proxied_by_defaultlets a domain default all managed records to proxied; per-recordproxied: truealso works. Gated on the provider'sproxycapability.HTTP API (RequireAuth + permission checks + audit logging)
GET /dns-records/ownership— per-record state (not_found | unmanaged | owned | owned_by_other) for the conflict UIPOST /dns-records— ownership-guarded create/update (audit:MANAGED_DNS_RECORD_SET)DELETE /dns-records— ownership-guarded removal (audit:MANAGED_DNS_RECORD_REMOVED)POST /dns-records/import— explicit adoption (audit:MANAGED_DNS_RECORD_IMPORTED)Also
dns_instance_identitysingle-row table: install-scoped random ID (CSPRNG uuid v4, deliberately NOT the telemetryanonymous_id) so two temps installs sharing a zone refuse to touch each other's records.DnsRecordServiceis untouched — it remains only for ACME challenge TXT records temps unambiguously owns. The legacy CLI-setup Cloudflare path is out of scope (runs pre-provider-config).Review history
An orchestrated review (security-auditor + Rust correctness pass) on the first commit found 3 invariant violations — registry-TXT clobber via upsert, name-only (not type-scoped) ownership, non-injective wildcard escaping — all fixed in the second commit with regression tests.
Testing
temps-dns(36 for this feature): marker parse/tamper/injection cases, clobber refusal (record + registry name), foreign-orphan refusal, our-orphan reuse, type-scoped ownership, injective escaping, proxy capability + depth gate, keyed-lock serialization/cleanup, import flows,instance_idget-or-create including insert-race recovery (MockDatabase).cargo check --libclean across the workspace; clippy clean (direct binary).Remaining before un-drafting
Related: #139, #146 (flat hostname strategy — merge prerequisite for the proxied path)