feat(nhi): per-identity DNS domain allowlists enforced at DoH - #71
Merged
Conversation
Manager side: machine_clients and oidc_trust_anchors now accept allowed_domains (JSON list of FQDNs or *.suffix patterns; NULL=unrestricted, []=deny all); CRUD validates hostnames, max 256 entries. Tokens issued via client_credentials and token-exchange include dns_domains claim ONLY when allowed_domains is non-NULL. DNS side: DoH query path enforces dns_domains claim with case-insensitive, trailing-dot-normalized matching (exact or wildcard suffix; *.example.com matches subdomains but NOT example.com itself). Non-match returns Status 3 (NXDOMAIN). Tokens without the claim behave exactly as today (zero regression). Metrics: dns_policy_denials counter labeled by outcome (policy_denied). Migration 010 adds allowed_domains columns (TEXT, JSON format) to both tables. Tests: manager CRUD validation, claim presence/absence, oversized list rejection; DNS matching exact/wildcard/depth/case/trailing-dot normalization, empty policy, absent claim. Both suites green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @PenguinzTech, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
- dns-server/app/main.py: combined imports -- this branch's domain_policy/jwt_verify additions plus already-merged #65's rate-limit config vars and RateLimiter import. Also removed a redundant local re-import of verify_squawk_jwt/JWT_PUBLIC_KEY (leftover from #65's original code) that was shadowing the combined module-level import and tripping flake8's F401 check. - dns-server/app/services/prometheus_metrics.py: combined two independent new methods (record_policy_denial from this branch, record_rate_limited_query from #65) on the same metrics class -- their backing Counter definitions survived the auto-merge intact. - manager/backend/app/services/auth_service.py: auto-merged cleanly but silently dropped the `List` import from typing while THIS branch's new allowed_domains: Optional[List[str]] parameter still uses it -- classic silent-auto-merge risk. Restored. Full manager suite: 264/264 passing. dns-server: 112/112 real tests passing (8 rate-limiter test errors are a pre-existing bug already fixed in #77, not yet in this branch's v2.1.x snapshot -- will re-verify after pulling latest v2.1.x). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
- app/blueprints/auth.py: both grant handlers (client_credentials and token-exchange) needed to combine THIS branch's dpop_jkt binding with #71's already-merged allowed_domains DB-fetch, both passed to create_machine_access_token. - app/services/auth_service.py: create_machine_access_token's signature/docstring/body combined to accept and apply both dpop_jkt (RFC 9449 cnf claim) and allowed_domains (dns_domains claim). - app/schema.py + tests/test_schema.py: dpop_replay table (this branch) combined with audit_event (#61, already merged) -- this branch predates that merge so git saw it as new. - alembic: two real chain issues, both from this branch predating later merges: 1. 011_dpop_replays.py had NO revision/down_revision identifiers at all (a genuine bug -- alembic could never have discovered this migration). Added revision="011_dpop_replays", down_revision="010_dns_domain_allowlists" (010 is #71's migration, merged after this branch was authored; the file's stale docstring said "Depends on: 009"). 2. 008_add_mfa_fields (re-chained onto 009 in an earlier standalone v2.1.x fix, before #71 existed) and 010_dns_domain_allowlists both forked from 009, creating two heads. Re-chained 008_add_mfa_fields -> 011_dpop_replays so the graph is linear again with 008_add_mfa_fields as the single head. Full manager suite: 293/293 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
- .gitleaks.toml: both branches independently created this file (add/add conflict). Kept origin/v2.1.x's version -- accumulated and already validated across #69/#71/#73/#77's resolutions. - .pre-commit-config.yaml: same flake8/hadolint divergence resolved identically to prior merges this session (single .flake8 source of truth + flake8-bugbear; hadolint v2.14.0). - app/schema.py: auth_user gained sso_provider/sso_subject (this branch) alongside external_id (#73 SCIM, already merged) -- both independent new columns, combined. - app/services/scopes.py: sso:write/sso:admin (this branch) combined with audit:read (already merged, SystemAdmin-only). - tests/test_schema.py: expected-tables set now covers sso_providers/ sso_login_attempts alongside scim_tokens/machine_client/ oidc_trust_anchor/dpop_replay/audit_event (all already merged). - alembic: SSO's own chain (008_add_mfa_fields -> 009_add_sso_providers -> 010_add_sso_login_attempts -> 011_allow_null_password_hash) and SCIM's chain (008_add_mfa_fields -> 010_add_scim_provisioning) both forked from the same parent, authored independently off feature/mfa-totp. Re-chained 009_add_sso_providers to depend on 010_add_scim_provisioning (SCIM's already-merged migration) instead, restoring a single linear head. Full manager suite: 339/339 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Per-identity DNS egress control for non-human identities — the agentic-security differentiator on top of #63. Machine clients and OIDC trust anchors gain
allowed_domains(NULL = unrestricted,[]= deny all, else exact FQDNs /*.suffixwildcards, validated, max 256). Tokens from both grants carry adns_domainsclaim only when a list is set; the DoH data plane enforces it after zone checks with a pure matching helper (case-insensitive, trailing-dot normalized, wildcard matches any depth but not the bare suffix) — non-match returns NXDOMAIN consistent with existing selective-router denials. Tokens without the claim behave exactly as today (legacy regression-tested). Denial counter labeled by outcome only — no domains or identities in metrics.Tests: 37 manager + 17 dns-server green (9 new CRUD/claim tests, 17 matching cases).
Stack note: Part 2 of the NHI stack — depends on #63 (
feature/nhi-machine-tokens); auto-retargets as the stack merges.🤖 Generated with Claude Code