Skip to content

fix(security): fail closed on unset database and Redis credentials - #1644

Closed
FenjuFu wants to merge 2 commits into
iflytek:mainfrom
FenjuFu:security/harden-default-db-credentials
Closed

fix(security): fail closed on unset database and Redis credentials#1644
FenjuFu wants to merge 2 commits into
iflytek:mainfrom
FenjuFu:security/harden-default-db-credentials

Conversation

@FenjuFu

@FenjuFu FenjuFu commented Aug 26, 2026

Copy link
Copy Markdown
Member

What

The bundled compose stack shipped well-known default credentials that the documented cp .env.example .env deployment path used verbatim:

  • PostgreSQL superuser spark123 and MySQL superuser root123, via ${VAR:-default} fallbacks in compose plus the same values set in .env.example.
  • Redis enabled --requirepass only when REDIS_PASSWORD was non-empty, and .env.example shipped the weak value 123 — so the default deployment ran Redis with a known password (or none, if the operator cleared it).

This aligns those credentials with the fail-fast preflight already used for MinIO/OSS.

Changes

docker/astronAgent/docker-compose.yaml

  • Database credentials (POSTGRES_PASSWORD, MYSQL_ROOT_PASSWORD, MYSQL_PASSWORD, DATABASE_PASSWORD): ${VAR:-weak-default}${VAR:?message}.
  • Redis: server command now redis-server --requirepass ${REDIS_PASSWORD:?…} (password mandatory, no longer conditional), and every consumer's REDIS_PASSWORD reference is fail-fast.

docker/astronAgent/.env.example

  • Blank POSTGRES_PASSWORD / MYSQL_ROOT_PASSWORD / REDIS_PASSWORD (with a short comment, same style as the MinIO block), and drop the :-root123 fallbacks from the chained MYSQL_PASSWORD / DATABASE_PASSWORD.

Why change both files

The compose ${VAR:-default} fallback only triggers when the variable is unset. Because .env.example set the weak values explicitly, changing compose alone would have no effect on the default deployment — both files must change together.

Validation

  • docker-compose.yaml remains valid YAML (16 services parse).
  • Compose variable semantics verified: an unset/empty credential aborts docker compose up with the guidance message; a set value resolves normally.

Out of scope (follow-up, needs coordination)

Service-to-service auth secrets (APP_AUTH_API_KEY/APP_AUTH_SECRET, COMMON_API_SECRET, TENANT_KEY/TENANT_SECRET) still carry placeholder defaults. These are validated by the core-tenant service against provisioned key data, so making the client side fail-fast requires updating the tenant-side provisioning in lockstep; converting them here would break the default deployment. Left for a coordinated follow-up. WORKFLOW_INTERNAL_API_KEY intentionally keeps its placeholder default — the auth layer treats the placeholder as "not configured" and returns 503, so it is already fail-closed.

The compose stack shipped default PostgreSQL (spark123) and MySQL (root123)
superuser passwords through ${VAR:-default} fallbacks, and .env.example set
those same values, so the documented `cp .env.example .env` deployment path
started with well-known credentials.

Convert every database-credential reference to the ${VAR:?message} fail-fast
form, mirroring the existing MinIO/OSS preflight, and blank the shipped values
in .env.example so the stack refuses to start until the operator provides a
strong, unique password.

Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
The bundled Redis enabled --requirepass only when REDIS_PASSWORD was
non-empty, and .env.example shipped the weak value `123`, so the documented
`cp .env.example .env` deployment ran Redis with a well-known password (or
with none, if the operator cleared it) on the shared network.

Make the password mandatory: the redis command and every consumer now use the
${VAR:?message} fail-fast form, and .env.example ships REDIS_PASSWORD empty so
the stack refuses to start until the operator sets a strong, unique value.

Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
@FenjuFu FenjuFu changed the title fix(security): fail closed on unset database credentials fix(security): fail closed on unset database and Redis credentials Aug 26, 2026
@FenjuFu

FenjuFu commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

Verification / validation notes for reviewers

This change is configuration-only (docker-compose.yaml + .env.example), so it was validated by parsing and by simulating Compose's variable semantics rather than by unit tests.

What was checked

  • docker-compose.yaml still parses as valid YAML — all 16 services load, and redis.command resolves to redis-server --requirepass ${REDIS_PASSWORD:?…}.
  • Compose ${VAR:?message} semantics confirmed for every converted credential:
    • unset / empty → docker compose up aborts with the guidance message (e.g. Set POSTGRES_PASSWORD to a strong, unique value).
    • set → resolves to the provided value, unchanged behavior.
  • No weak default remains: a scan for :-root123 / :-spark123 / REDIS_PASSWORD=123 and the blanked .env.example values returns nothing.

Why both files change together

The Compose ${VAR:-default} fallback only triggers when the variable is unset. .env.example previously set the weak values explicitly, so editing compose alone would not affect the default cp .env.example .env path — both sides must change for the guard to take effect.

Scope note

Database + Redis credentials are covered here. The service-to-service auth secrets (APP_AUTH_API_KEY/APP_AUTH_SECRET, COMMON_API_SECRET, TENANT_KEY/TENANT_SECRET) are intentionally left out: they are validated by core-tenant against provisioned key data, so making the client side fail-fast requires a coordinated tenant-side change and would otherwise break the default deployment. WORKFLOW_INTERNAL_API_KEY keeps its placeholder default on purpose — the auth layer treats the placeholder as "not configured" and returns 503, so it is already fail-closed.

@FenjuFu
FenjuFu requested a review from lyj715824 August 28, 2026 13:55
@lyj715824

Copy link
Copy Markdown
Contributor

该做法会增加用户的部署复杂度,建议提供初始化密码,用户根据需要修改;https://github.com/iflytek/astron-agent/security/advisories/GHSA-mh3w-4q3f-2fg5 报告中提到的其他问题,基本已修复;

@lyj715824

Copy link
Copy Markdown
Contributor

此 PR我先关闭

@lyj715824 lyj715824 closed this Aug 30, 2026
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.

2 participants