Skip to content

fix: keep secrets out of restore logs and mutation responses - #28

Merged
agentHits merged 4 commits into
AgentHits-Devfrom
fix/response-and-log-secrets
Oct 7, 2026
Merged

agentHits merged 4 commits into
AgentHits-Devfrom
fix/response-and-log-secrets

Conversation

@agentHits

Copy link
Copy Markdown
Collaborator

What is this PR about?

Stops secrets from leaking through log text and mutation responses. Four commits, one per area:

  1. Database restore and password-change errors. On a local server a failed mysql/mariadb restore streams Error: <command text> from backup.restoreBackupWithLogs, and the command held the password as mysql -u root -p<password>, which redactSensitiveText did not mask. The same form was used by mysql/mariadb changePassword for the root password, and that error text goes back to the caller.
    • The password now reaches the container via docker exec -e DOKPLOY_DB_PASSWORD=... and the client reads it as -p"$DOKPLOY_DB_PASSWORD".
    • redactSensitiveText masks a value glued to -p after mysql/mariadb client names. -p followed by a space, uppercase -P (port) and other tools' -p (docker, ssh, compose, mkdir) are left alone.
    • Env assignments and --password style flags are now masked in full even when shell-quote escaped &, | or quotes in the value. Before, a password with & or | (both allowed by the schema) was masked only up to that character.
  2. Compose previews. compose.randomizeCompose, compose.isolatedDeployment and compose.getConvertedCompose now return the compose file only to callers with envVars.read. Other callers get __DOKPLOY_REDACTED_SECRET__. This is the same rule compose.one uses for composeFile.
  3. Shared env in mutation responses.
    • environment.update and environment.remove return env only with environmentEnvVars.read.
    • project.update and project.remove return env only with projectEnvVars.read.
    • project.duplicate hides env and project.env by the same permissions. This matters most with duplicateInSameProject, where the response is the source environment.
    • The env and project-env redaction is now in redactEnvironmentSharedEnvFor, which service reads also use.
  4. Member listings. user.all and user.one select only the user profile fields the UI reads: id, names, email, emailVerified, image, banned, twoFactorEnabled and createdAt. Before, they returned the whole user row, including licenseKey, Stripe ids, trustedOrigins and the ban reason and expiry.

What changes for each role:

  • Owners and admins: nothing visible.
  • Members without env read permissions: they see the placeholder in compose previews, and env values are redacted in these mutation responses.
  • API clients: fields outside the member profile whitelist are no longer returned by user.all and user.one.

Checks

  • CI=true bunx vitest run --config __test__/vitest.config.ts on the security, backup, backups, compose, environment, placement, users and database folders: 73 files passed, 1 skipped (518 tests passed).
  • Full suite: only __test__/deploy/application.real.test.ts fails, as expected locally (no Docker socket).
  • Each new or updated test fails on the original code. I checked this by reverting the source files to origin/AgentHits-Dev and re-running: 16 failures.
  • Two tests run the generated restore and password-change commands through a docker stub. They confirm that mysql and mariadb still receive -p<password> and the ALTER USER statement.
  • bun run typecheck: exit 0.
  • bunx biome check <changed files>: 0 errors.

Rollout notes

  • mysql and mariadb restore and password change now call the client through sh -c inside the container (the official images include sh). These were not run against a live database container.
  • Restore and password-change logs now show DOKPLOY_DB_PASSWORD=__DOKPLOY_REDACTED_SECRET__ where they used to show the raw -p<password>.
  • No migrations or settings changes.

🤖 Generated with Claude Code

agentHits and others added 4 commits October 7, 2026 00:53
…nge errors

Что:
- Restore mysql/mariadb и changePassword для mysql/mariadb передают пароль в контейнер через `docker exec -e DOKPLOY_DB_PASSWORD=...`, клиент читает его как `-p"$DOKPLOY_DB_PASSWORD"`.
- `redactSensitiveText` маскирует пароль, приклеенный к `-p` после вызова mysql/mariadb-клиентов (`-p` с пробелом, `-P` порт и `-p` других утилит не трогаются).
- Значения в env-присваиваниях и `--password` флагах маскируются целиком, даже если shell-quote экранировал в них `&`, `|` или кавычки.
Зачем:
- При локальном restore текст упавшей команды попадает в `Error: ...`, который стримит `backup.restoreBackupWithLogs`, а форма `-p<пароль>` не маскировалась. То же касалось ошибок changePassword (root-пароль).
- Пароли с `&` или `|` (разрешены схемой) раньше маскировались только до этого символа.
Риски:
- Restore и смена пароля mysql/mariadb теперь вызывают клиента через `sh -c` внутри контейнера; в официальных образах mysql/mariadb `sh` есть. В логах вместо `-p<пароль>` теперь видно `DOKPLOY_DB_PASSWORD=__DOKPLOY_REDACTED_SECRET__`.
- Новое правило маскирует `-p<значение>` после слов mysql*/mariadb* в той же строке, редкий не-парольный `-pXXX` после них тоже будет скрыт.
Проверки:
- Команды и результаты: `CI=true bunx vitest run --config __test__/vitest.config.ts` по новым/изменённым тестам (database-password-command-redaction, secret-redaction, change-password-command-boundary, restore-security) - passed; эти тесты падают на исходном коде (проверено откатом исходников к origin/AgentHits-Dev). `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 ошибок.
- Ограничения: реальный restore и смена пароля в живом mysql/mariadb-контейнере не запускались; вызов клиента проверен stub-ом `docker`, который применяет `-e` и запускает внутренний `sh -c`.

What:
- mysql/mariadb restore and mysql/mariadb changePassword pass the password to the container via `docker exec -e DOKPLOY_DB_PASSWORD=...`; the client reads it as `-p"$DOKPLOY_DB_PASSWORD"`.
- `redactSensitiveText` masks a password glued to `-p` after mysql/mariadb client invocations (`-p` followed by a space, `-P` port and other tools' `-p` are left alone).
- Values in env assignments and `--password` flags are masked whole even when shell-quote escaped `&`, `|` or quotes in them.
Why:
- On local restores the failed command text ends up in the `Error: ...` line streamed by `backup.restoreBackupWithLogs`, and the `-p<password>` form was not masked. changePassword errors had the same problem with the root password.
- Passwords with `&` or `|` (allowed by the schema) were masked only up to that character.
Risks:
- mysql/mariadb restore and password change now run the client through `sh -c` inside the container; official mysql/mariadb images ship `sh`. Logs now show `DOKPLOY_DB_PASSWORD=__DOKPLOY_REDACTED_SECRET__` instead of `-p<password>`.
- The new rule masks `-p<value>` after mysql*/mariadb* words on the same line, so a rare non-password `-pXXX` after them is hidden too.
Checks:
- Commands and results: `CI=true bunx vitest run --config __test__/vitest.config.ts` on the new/changed tests (database-password-command-redaction, secret-redaction, change-password-command-boundary, restore-security) - passed; these tests fail on the original code (checked by reverting the sources to origin/AgentHits-Dev). `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 errors.
- Limitations: no real restore or password change against a live mysql/mariadb container; the client invocation is checked with a `docker` stub that applies `-e` and runs the inner `sh -c`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Что:
- `compose.randomizeCompose`, `compose.isolatedDeployment` и `compose.getConvertedCompose` возвращают compose-файл только при праве `envVars.read`; иначе вместо файла приходит `__DOKPLOY_REDACTED_SECRET__`.
Зачем:
- `compose.one` уже скрывает `composeFile` от пользователей без `envVars.read`, но эти три процедуры отдавали тот же файл (часто с inline-секретами) любому с `service.create`.
Риски:
- Пользователь без `envVars.read` видит в превью randomize/isolated/converted compose плейсхолдер вместо YAML. Ошибок в UI нет, сохранение и деплой не меняются; owner/admin и роли с `envVars.read` видят файл как раньше.
Проверки:
- Команды и результаты: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/compose/compose-file-preview-secrets.test.ts` - passed, падает на исходном коде (проверено откатом compose.ts к origin/AgentHits-Dev); папка `__test__/compose` - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 ошибок.
- Ограничения: UI вручную не открывался.

What:
- `compose.randomizeCompose`, `compose.isolatedDeployment` and `compose.getConvertedCompose` return the compose file only with `envVars.read`; otherwise the result is `__DOKPLOY_REDACTED_SECRET__`.
Why:
- `compose.one` already hides `composeFile` from users without `envVars.read`, but these three procedures returned the same file (often with inline secrets) to anyone with `service.create`.
Risks:
- A user without `envVars.read` sees the placeholder instead of YAML in the randomize/isolated/converted compose previews. No UI errors; saving and deploys are unchanged; owners/admins and roles with `envVars.read` see the file as before.
Checks:
- Commands and results: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/compose/compose-file-preview-secrets.test.ts` - passed, fails on the original code (checked by reverting compose.ts to origin/AgentHits-Dev); the `__test__/compose` folder - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 errors.
- Limitations: the UI was not opened manually.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Что:
- `environment.update` и `environment.remove` возвращают `env` только при праве `environmentEnvVars.read`, иначе плейсхолдер.
- `project.update` и `project.remove` возвращают `env` только при праве `projectEnvVars.read`.
- `project.duplicate` (особенно с `duplicateInSameProject`, где возвращается исходное окружение) скрывает `env` и `project.env` по тем же правам.
- Логика "env окружения + env проекта по правам" вынесена в `redactEnvironmentSharedEnvFor` и переиспользуется в чтениях сервисов.
Зачем:
- Мутации отдавали сырые общие переменные окружения и проекта пользователям, у которых нет права их читать (чтения `environment.one`/`project.one` их уже скрывают).
Риски:
- UI использует из ответов только id и name, поэтому видимых изменений нет. Внешние API-клиенты без прав чтения env получат плейсхолдер вместо значения в ответах этих мутаций.
Проверки:
- Команды и результаты: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/environment/environment-secret-boundary.test.ts __test__/security/project-env-response-redaction.test.ts __test__/placement/placement-ownership-boundary.test.ts` - passed; новые тесты падают на исходном коде (проверено откатом роутеров к origin/AgentHits-Dev). Папки security, environment, placement - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 ошибок.
- Ограничения: в placement-ownership-boundary только добавлен мок `hasPermission`, который теперь вызывают мутации.

What:
- `environment.update` and `environment.remove` return `env` only with `environmentEnvVars.read`, otherwise the placeholder.
- `project.update` and `project.remove` return `env` only with `projectEnvVars.read`.
- `project.duplicate` (notably with `duplicateInSameProject`, which returns the source environment) hides `env` and `project.env` by the same permissions.
- The "environment env + project env by permission" logic moved into `redactEnvironmentSharedEnvFor` and is reused by service reads.
Why:
- These mutations returned raw shared environment and project variables to users without permission to read them (the `environment.one`/`project.one` reads already hide them).
Risks:
- The UI only uses ids and names from these responses, so nothing visible changes. External API clients without env read permission get the placeholder in these mutation responses.
Checks:
- Commands and results: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/environment/environment-secret-boundary.test.ts __test__/security/project-env-response-redaction.test.ts __test__/placement/placement-ownership-boundary.test.ts` - passed; the new tests fail on the original code (checked by reverting the routers to origin/AgentHits-Dev). The security, environment and placement folders - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 errors.
- Limitations: placement-ownership-boundary only gains a `hasPermission` mock that the mutations now call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Что:
- `user.all` и `user.one` загружают у связанного пользователя только поля профиля: id, firstName, lastName, email, emailVerified, image, banned, twoFactorEnabled, createdAt.
Зачем:
- Раньше возвращалась вся строка user: licenseKey, stripeCustomerId, stripeSubscriptionId, trustedOrigins, banReason/banExpires и прочие настройки владельца аккаунта - любому с правом member.read или самому участнику.
Риски:
- UI (список пользователей, выбор пользователя в сессиях, права участника) читает только оставшиеся поля. Внешние API-клиенты, которые брали из этих ответов служебные поля пользователя, их больше не получат.
Проверки:
- Команды и результаты: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/users/user-member-listing-fields.test.ts` - passed, падает на исходном коде (проверено откатом user.ts к origin/AgentHits-Dev); папка `__test__/users` - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 ошибок.
- Ограничения: выборка колонок в тесте эмулируется моком, реальный drizzle-запрос к базе не выполнялся.

What:
- `user.all` and `user.one` load only profile fields of the related user: id, firstName, lastName, email, emailVerified, image, banned, twoFactorEnabled, createdAt.
Why:
- The full user row was returned before: licenseKey, stripeCustomerId, stripeSubscriptionId, trustedOrigins, banReason/banExpires and other account-owner settings, to anyone with member.read or to the member themselves.
Risks:
- The UI (users list, session user picker, member permissions) only reads the remaining fields. External API clients that took internal user fields from these responses no longer get them.
Checks:
- Commands and results: `CI=true bunx vitest run --config __test__/vitest.config.ts __test__/users/user-member-listing-fields.test.ts` - passed, fails on the original code (checked by reverting user.ts to origin/AgentHits-Dev); the `__test__/users` folder - passed. `bun run typecheck` - exit 0; `bunx biome check <changed files>` - 0 errors.
- Limitations: the column selection is emulated by the test mock; no real drizzle query ran against a database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@agentHits
agentHits merged commit 2051ec5 into AgentHits-Dev Oct 7, 2026
8 checks passed
@agentHits
agentHits deleted the fix/response-and-log-secrets branch October 7, 2026 05:51
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