Repository navigation
fix: keep secrets out of restore logs and mutation responses - #28
Merged
Merged
Conversation
…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>
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
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.
What is this PR about?
Stops secrets from leaking through log text and mutation responses. Four commits, one per area:
Error: <command text>frombackup.restoreBackupWithLogs, and the command held the password asmysql -u root -p<password>, whichredactSensitiveTextdid not mask. The same form was used by mysql/mariadbchangePasswordfor the root password, and that error text goes back to the caller.docker exec -e DOKPLOY_DB_PASSWORD=...and the client reads it as-p"$DOKPLOY_DB_PASSWORD".redactSensitiveTextmasks a value glued to-pafter mysql/mariadb client names.-pfollowed by a space, uppercase-P(port) and other tools'-p(docker, ssh, compose, mkdir) are left alone.--passwordstyle 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.compose.randomizeCompose,compose.isolatedDeploymentandcompose.getConvertedComposenow return the compose file only to callers withenvVars.read. Other callers get__DOKPLOY_REDACTED_SECRET__. This is the same rulecompose.oneuses forcomposeFile.environment.updateandenvironment.removereturnenvonly withenvironmentEnvVars.read.project.updateandproject.removereturnenvonly withprojectEnvVars.read.project.duplicatehidesenvandproject.envby the same permissions. This matters most withduplicateInSameProject, where the response is the source environment.redactEnvironmentSharedEnvFor, which service reads also use.user.allanduser.oneselect 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:
user.allanduser.one.Checks
CI=true bunx vitest run --config __test__/vitest.config.tson the security, backup, backups, compose, environment, placement, users and database folders: 73 files passed, 1 skipped (518 tests passed).__test__/deploy/application.real.test.tsfails, as expected locally (no Docker socket).origin/AgentHits-Devand re-running: 16 failures.dockerstub. They confirm that mysql and mariadb still receive-p<password>and theALTER USERstatement.bun run typecheck: exit 0.bunx biome check <changed files>: 0 errors.Rollout notes
sh -cinside the container (the official images includesh). These were not run against a live database container.DOKPLOY_DB_PASSWORD=__DOKPLOY_REDACTED_SECRET__where they used to show the raw-p<password>.🤖 Generated with Claude Code