Skip to content

fix(deploy): require ATP_MASTER_KEY in docker-compose instead of defaulting to an invalid value - #235

Merged
argszero merged 1 commit into
mainfrom
fix/compose-master-key-required
Sep 14, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/compose-master-key-required

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

docker-compose.yml shipped an invalid default for ATP_MASTER_KEY; make it a required variable
instead.

Host report (rant 2026-09-14T17:30:56).

The file set:

- ATP_MASTER_KEY=${ATP_MASTER_KEY:-dev-master-key-请替换}

That default is not a 64-hex string (18 chars, containing - and CJK). crypto::parse_master_key
rejects it, and Crypto::from_config logs an error and falls through to the random dev key:

// src/crypto.rs
Err(e) => { log::error!("ATP_MASTER_KEY 无效(需 32 字节 hex): {e},回退下一来源"); }   // :41
...
OsRng.fill_bytes(&mut key);                                                          // :56-58

So the container starts happily, encrypts upstream keys with a random key, and after the first
restart every already-encrypted upstream key is undecryptable - every upstream call returns 503. The
symptom looks like a provider/key problem, not a configuration one.

Related Issue

No issue exists for this - it was reported directly by the host. Left empty on purpose rather than
fabricating one.

Changes

  • docker-compose.yml:
    • the environment entry becomes - ATP_MASTER_KEY=${ATP_MASTER_KEY:?未设置 ATP_MASTER_KEY:…} -
      compose refuses to start and prints the generation command;
    • the quick start shows the required export ATP_MASTER_KEY=$(openssl rand -hex 32);
    • the header comment "未设置时使用示例默认值(…)" is corrected - it described a value that can
      never be parsed, i.e. behaviour that does not exist.
  • Deliberately not a valid-hex default. A hard-coded hex default would turn "breaks on restart"
    into "every deployment shares one public master key", which is strictly worse.

Change radius = 1 file. Carrier scan (git grep over tracked files): docker-compose.yml is the
only artifact that sets a value. Dockerfile:5, README.md:37, README.en.md:37,
docs/architecture.md:88 and config/config.example.toml:20 all only show openssl rand -hex 32
examples or describe the env var. No dev-master-key reference remains.

  • Config/data-structure changes are mirrored into the example file (this is a compose file, not
    app config - no example-file counterpart)

Tests

  • cargo test unchanged - 245 passed / 0 failed (this file is deployment glue; it is not
    compiled and no test reads it)

  • cargo fmt --check exit 0

  • New tests added (n/a - no Rust code changed)

  • Local pre-validation (YAML parse + an explicit model of compose interpolation, not docker
    itself):

    check result
    both documents parse, environment entry present PASS
    the new entry uses ${VAR:?msg} PASS
    no non-hex :- default survives PASS
    negative control: the old default is not valid hex (len 18, first bad char at index 2) PASS
    model: unset → error PASS
    model: empty → error PASS
    model: set to 64 hex → ok PASS
  • ⚠️ Authoritative check still pending: docker compose config must fail without the variable and
    succeed with it. Docker is not installed on this machine (docker: command not found), so this
    has to run in a docker-capable environment or CI. The local probe is explicitly a model of the
    interpolation, not the tool.

Checklist

  • Branch naming follows the convention: fix/compose-master-key-required
  • Commit message uses Conventional Commits
  • Single responsibility, minimal change

Deployment note

Anyone running docker compose up from this file must now set ATP_MASTER_KEY first
(export ATP_MASTER_KEY=$(openssl rand -hex 32)); without it compose stops with a clear message
instead of silently starting with a broken key. Existing deployments that already pass the variable
are unaffected.

…ulting to an invalid value

Host report (rant 2026-09-14T17:30:56).

`docker-compose.yml` set `ATP_MASTER_KEY=${ATP_MASTER_KEY:-dev-master-key-请替换}`.
That default is not a 64-hex string (18 chars; contains `-` and CJK characters),
so `crypto::parse_master_key` rejects it and `Crypto::from_config` logs an error
and **falls through** (src/crypto.rs:41) to the random dev key (src/crypto.rs:56-58).
A restart then makes every already-encrypted upstream key undecryptable, i.e.
every upstream call returns 503 - a silent full outage that looks like a key
problem rather than a configuration one.

Fail loud instead of shipping a default: `${ATP_MASTER_KEY:?msg}` makes
`docker compose` refuse to start and print the generation command. This is
deliberately **not** a valid-hex default - that would trade "breaks on restart"
for "every deployment shares one public master key".

The header comment claiming "未设置时使用示例默认值" is corrected too: the value it
described could never be parsed, so the comment documented behaviour that does
not exist.

- docker-compose.yml: the environment entry uses `${ATP_MASTER_KEY:?…}`, the
  quick-start shows the required `export`, and the misleading comment is fixed.
  This is the only tracked artifact that *sets* a value; Dockerfile, READMEs,
  README.en.md, docs/architecture.md and config.example.toml all only show
  `openssl rand -hex 32` examples or describe the env var.

Tests: `cargo test` unchanged at 245 passed / 0 failed (this file is deployment
glue and is not compiled). `cargo fmt --check` exit 0. Verified locally by
parsing the YAML and running an explicit model of compose interpolation
(`${VAR:?}` / `${VAR:-def}`): unset -> error, empty -> error, set-valid -> ok;
the old default is confirmed non-hex. `docker compose config` is the
authoritative check but docker is not installed on this machine, so that step
is left to a docker-capable environment / CI.
@argszero
argszero merged commit 7852277 into main Sep 14, 2026
1 check passed
@argszero
argszero deleted the fix/compose-master-key-required branch September 14, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant