From 89a6f667af1a77dcbc2340f884f901a02e2d51b8 Mon Sep 17 00:00:00 2001 From: Derek Date: Sun, 15 Nov 2026 10:13:45 +1100 Subject: [PATCH] fix(secrets): mint passwords and keys from letters and digits The ClickHouse service, org and group passwords and the SCIM provisioning password are 32 letters and digits. The CLI's throwaway JWT key is 64 letters and digits. They were minted with token_urlsafe, whose - and _ get rewritten by shells, .env files and URL-form DSNs. Only mint_value in dfe_engine/secrets.py mints them. Session epochs and bearer tokens stay as they are. --- src/dfe_engine/auth/scim_mapping.py | 4 ++-- src/dfe_engine/cli/auto/app.py | 4 ++-- src/dfe_engine/governance/ch/reconciler.py | 4 ++-- src/dfe_engine/secrets.py | 19 +++++++++++++++++++ tests/unit/test_secrets_interface.py | 14 ++++++++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/dfe_engine/auth/scim_mapping.py b/src/dfe_engine/auth/scim_mapping.py index f02474bca..73e03f11e 100644 --- a/src/dfe_engine/auth/scim_mapping.py +++ b/src/dfe_engine/auth/scim_mapping.py @@ -29,7 +29,6 @@ SCIM face so downstream tooling can tell IdP-owned records apart. """ -import secrets from datetime import datetime from typing import TypedDict @@ -39,6 +38,7 @@ from dfe_engine.auth.accounts import Account from dfe_engine.auth.groups import Group +from dfe_engine.secrets import mint_value # Marker written to ``source_provider`` for anything the SCIM face provisions. SCIM_SOURCE_PROVIDER = "scim" @@ -126,7 +126,7 @@ def generate_provisioning_password() -> str: IdP-provisioned users authenticate via the IdP, not a local password, but the store requires a bcrypt-hashable secret. This mints one they never use. """ - return secrets.token_urlsafe(32) + return mint_value() # -- Group <-> SCIM Group ------------------------------------- diff --git a/src/dfe_engine/cli/auto/app.py b/src/dfe_engine/cli/auto/app.py index eb44bc5ff..cc3e31d09 100644 --- a/src/dfe_engine/cli/auto/app.py +++ b/src/dfe_engine/cli/auto/app.py @@ -16,7 +16,6 @@ import os import re -import secrets from typing import Any from scalo.cli.openapi import ScaloHttpTransport, SpecCli, Store, Transport @@ -34,8 +33,9 @@ def load_live_spec() -> dict[str, Any]: jwt_secret, which a schema Job does not carry. A throwaway one satisfies that check. """ from dfe_engine.api.app import create_app + from dfe_engine.secrets import KEY_LENGTH, mint_value - os.environ.setdefault("DFE_ENGINE_API_JWT_SECRET", secrets.token_urlsafe(32)) + os.environ.setdefault("DFE_ENGINE_API_JWT_SECRET", mint_value(KEY_LENGTH)) return create_app().openapi() diff --git a/src/dfe_engine/governance/ch/reconciler.py b/src/dfe_engine/governance/ch/reconciler.py index 372750033..8d2cac320 100644 --- a/src/dfe_engine/governance/ch/reconciler.py +++ b/src/dfe_engine/governance/ch/reconciler.py @@ -15,7 +15,6 @@ """ import hashlib -import secrets as _stdlib_secrets from collections.abc import Collection, Mapping from typing import Any @@ -26,6 +25,7 @@ from dfe_engine.clickhouse.errors import is_connection_error from dfe_engine.orgs.tenant_scope import org_tenant_ids from dfe_engine.schema.require import missing_objects +from dfe_engine.secrets import mint_value from .models import ( DB, @@ -304,7 +304,7 @@ def _hash_for(self, path: str) -> str: user's password, so regenerating every run would desync CH from the store. The mint is create-only, so two replicas reconciling together keep one password. """ - minted = _stdlib_secrets.token_urlsafe(24)[:32] + minted = mint_value() if self._secrets is None: return hashlib.sha256(minted.encode()).hexdigest() plaintext = self._secrets.get_or_none(path) diff --git a/src/dfe_engine/secrets.py b/src/dfe_engine/secrets.py index 439f1a943..b93eddfdc 100644 --- a/src/dfe_engine/secrets.py +++ b/src/dfe_engine/secrets.py @@ -18,6 +18,8 @@ The engine never imports a backend SDK - scalo owns that (see docs/deployment/backing-services.md). """ +import secrets as _stdlib_secrets +import string import time from collections.abc import Callable, Sequence from contextlib import suppress @@ -41,6 +43,23 @@ # AWS finishes a forced delete in the background, so a write to that name waits up to 30s for it. FORCED_DELETE_BACKOFF: tuple[float, ...] = (1.0, 2.0, 4.0, 8.0, 15.0) +# Letters and digits only, because shells, .env files and URL-form DSNs rewrite `$`, `-` and `_` +_MINT_ALPHABET = string.ascii_letters + string.digits +PASSWORD_LENGTH = 32 +KEY_LENGTH = 64 + + +def mint_value(length: int = PASSWORD_LENGTH) -> str: + """A random string of letters and digits. + + Args: + length: PASSWORD_LENGTH for a password, KEY_LENGTH for a signing key. + + Returns: + The minted value. + """ + return "".join(_stdlib_secrets.choice(_MINT_ALPHABET) for _ in range(length)) + class SecretNameHeldError(ProviderError): """A deleted secret that has not finished deleting holds a name the engine reads or writes.""" diff --git a/tests/unit/test_secrets_interface.py b/tests/unit/test_secrets_interface.py index 873e900b4..4813f2e51 100644 --- a/tests/unit/test_secrets_interface.py +++ b/tests/unit/test_secrets_interface.py @@ -23,9 +23,12 @@ from dfe_engine.secrets import ( FORCED_DELETE_BACKOFF, + KEY_LENGTH, + PASSWORD_LENGTH, DfeSecrets, SecretNameHeldError, build_secrets, + mint_value, ) from dfe_engine.settings import SecretsSettings @@ -197,3 +200,14 @@ def test_tags_reach_aws_only(provider, tags): _scripted_store(manager, provider=provider).create("ch/groups/soc", "pw") assert manager.created_tags == [tags] + + +@pytest.mark.parametrize( + ("length", "expected"), [((), 32), ((KEY_LENGTH,), 64), ((PASSWORD_LENGTH,), 32)] +) +def test_a_minted_value_is_letters_and_digits_at_its_length(length, expected): + """A `$`, `-` or `_` in a minted value would be rewritten by a shell, a .env file or a DSN.""" + values = {mint_value(*length) for _ in range(50)} + + assert len(values) == 50 + assert all(len(value) == expected and value.isascii() and value.isalnum() for value in values)