feat: add security hardening and monitoring updates#4708
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f89a8a8ac4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилил authorization boundaries в router/service/WSS слоях: service access, assigned-server access, organization scoping, permission checks и safer deploy-source ownership checks. - Добавил переиспользуемые access helpers для Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets и provider ownership. - Усилил command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization и SSRF-aware URL checks. - Добавил secret redaction для API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context и logs. - Закрыл Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding и safer custom Git URL handling. - Закрепил schedule/deployment worker jobs signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection и explicit package exports for signed schedule/deployment helpers. - Улучшил monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits и responsive layout. - Добавил safe application env upsert: dry-run metadata, revision checks, secret-aware result summaries и conflict protection. - Обновил DB/schema/tooling surface для SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path и server package export switch scripts with stable final newline output. - После Codex Review restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into new forward-only migration `0175_guard_host_schedule_scope`. - Добавил и актуализировал regression coverage across auth, backup, deploy, Docker, Git providers, monitoring, schedule/deployment signing, permissions, redaction, safe paths, webhooks, package exports and migration placement. Зачем: - Для владельца Dokploy это снижает риск IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets и stale worker job execution. - Для maintainer review это превращает большой набор security/UX gaps в проверяемые helpers, negative fixtures, focused tests и повторяемые package/build boundaries. - Для операторов Monitoring становится понятнее: видно, какие containers/images/volumes/processes используют ресурсы, а Docker Disk Usage раскрывается без потери контекста. - Для integrations safe env upsert дает менее рискованный путь точечно обновлять application environment variables без silent overwrite. - Для PR Dokploy#4708 review comments это закрывает runtime package-export gap for `@dokploy/server/utils/deployments/signed-job` and keeps already-applied migrations immutable. Риски: - PR commit остается большим и squashed; maintainer может попросить разделить security, monitoring и dependency/tooling parts на несколько PR. - Dependency/tooling refresh широкий, поэтому upstream CI после push остается финальным source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners, because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Полный локальный `pnpm test` after review fixes failed only in real deploy tests under `__test__/deploy/application.real.test.ts`; 151 files passed and 1328 tests passed, but 4 shell clone/build deploy cases failed through local `execAsync`. - Browser smoke был локальным unauthenticated boot/render smoke: login page, `/` HTTP 200 и screenshot; authenticated dashboard flows локально не проверялись. - Локальная среда использовала Node v24.18.0 при `.nvmrc` 24.4.0; engine range допускает 24.x, но это отличается от exact project recommendation. Проверки: - Команды и результаты: `git diff --exit-code upstream/canary -- apps/dokploy/drizzle/0169_parched_johnny_storm.sql` passed; `cmp -s apps/dokploy/drizzle/meta/0174_snapshot.json apps/dokploy/drizzle/meta/0175_snapshot.json` passed; `jq` checks for `_journal.json` and `0175_snapshot.json` passed; `CI=true corepack pnpm --dir apps/dokploy exec vitest --config __test__/vitest.config.ts run __test__/deployment/signed-job-scope.test.ts __test__/security/high-findings-regression.test.ts` passed twice, final run 2 files / 16 tests; `git diff --check` and `git diff --cached --check` passed; `CI=true corepack pnpm exec biome check apps/dokploy/__test__/deployment/signed-job-scope.test.ts apps/dokploy/__test__/security/high-findings-regression.test.ts packages/server/scripts/switchToDist.js packages/server/scripts/switchToSrc.js packages/server/package.json` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api and schedules; `CI=true corepack pnpm run server:build` passed; `test -f packages/server/dist/utils/deployments/signed-job.js` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `CI=true corepack pnpm run test` failed only in `__test__/deploy/application.real.test.ts`, with 151 files passed / 4 failed real deploy tests / 1328 tests passed / 1 skipped; independent Agent Flow reviewer.qa returned pass-with-risks with no blocker for amend/push. - Ограничения: upstream CI, maintainer review, authenticated browser flows, production deployment and a real DB migration fixture for `0175` were not run locally. What: - Strengthened authorization boundaries across router/service/WSS layers: service access, assigned-server access, organization scoping, permission checks, and safer deploy-source ownership checks. - Added reusable access helpers for Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets, and provider ownership. - Hardened command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization, and SSRF-aware URL checks. - Added secret redaction for API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context, and logs. - Closed Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding, and safer custom Git URL handling. - Bound schedule/deployment worker jobs with signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection, and explicit package exports for signed schedule/deployment helpers. - Improved monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits, and responsive layout. - Added safe application env upsert with dry-run metadata, revision checks, secret-aware result summaries, and conflict protection. - Updated DB/schema/tooling surface for SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path, and server package export switch scripts with stable final-newline output. - After Codex Review, restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into the new forward-only migration `0175_guard_host_schedule_scope`. - Added and updated regression coverage across auth, backup, deploy, Docker, Git providers, monitoring, schedule/deployment signing, permissions, redaction, safe paths, webhooks, package exports, and migration placement. Why: - For the Dokploy owner, this lowers the risk of IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets, and stale worker job execution. - For maintainer review, this turns a large set of security and UX gaps into reviewable helpers, negative fixtures, focused tests, and repeatable package/build boundaries. - For operators, Monitoring becomes clearer: containers/images/volumes/processes show resource ownership, and Docker Disk Usage can expand without losing context. - For integrations, safe env upsert provides a lower-risk way to patch application environment variables without silent overwrite. - For PR Dokploy#4708 review comments, this closes the runtime package-export gap for `@dokploy/server/utils/deployments/signed-job` and keeps already-applied migrations immutable. Risks: - The PR commit remains large and squashed; the maintainer may ask to split security, monitoring, and dependency/tooling parts into separate PRs. - The dependency/tooling refresh is broad, so upstream CI after push remains the final source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Full local `pnpm test` after review fixes failed only in real deploy tests under `__test__/deploy/application.real.test.ts`; 151 files passed and 1328 tests passed, but 4 shell clone/build deploy cases failed through local `execAsync`. - Browser smoke was a local unauthenticated boot/render smoke: login page, `/` HTTP 200, and screenshot; authenticated dashboard flows were not checked locally. - The local environment used Node v24.18.0 while `.nvmrc` is 24.4.0; the engine range accepts 24.x, but it differs from the exact project recommendation. Checks: - Commands and results: `git diff --exit-code upstream/canary -- apps/dokploy/drizzle/0169_parched_johnny_storm.sql` passed; `cmp -s apps/dokploy/drizzle/meta/0174_snapshot.json apps/dokploy/drizzle/meta/0175_snapshot.json` passed; `jq` checks for `_journal.json` and `0175_snapshot.json` passed; `CI=true corepack pnpm --dir apps/dokploy exec vitest --config __test__/vitest.config.ts run __test__/deployment/signed-job-scope.test.ts __test__/security/high-findings-regression.test.ts` passed twice, final run 2 files / 16 tests; `git diff --check` and `git diff --cached --check` passed; `CI=true corepack pnpm exec biome check apps/dokploy/__test__/deployment/signed-job-scope.test.ts apps/dokploy/__test__/security/high-findings-regression.test.ts packages/server/scripts/switchToDist.js packages/server/scripts/switchToSrc.js packages/server/package.json` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api, and schedules; `CI=true corepack pnpm run server:build` passed; `test -f packages/server/dist/utils/deployments/signed-job.js` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `CI=true corepack pnpm run test` failed only in `__test__/deploy/application.real.test.ts`, with 151 files passed / 4 failed real deploy tests / 1328 tests passed / 1 skipped; independent Agent Flow reviewer.qa returned pass-with-risks with no blocker for amend/push. - Limitations: upstream CI, maintainer review, authenticated browser flows, production deployment, and a real DB migration fixture for `0175` were not run locally.
|
@codex review |
|
Local verification update for
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 635a8048ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилил authorization boundaries в router/service/WSS слоях: service access, assigned-server access, organization scoping, permission checks и safer deploy-source ownership checks. - Добавил переиспользуемые access helpers для Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets и provider ownership. - Усилил command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization и SSRF-aware URL checks. - Добавил secret redaction для API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context и logs. - Сохранил server-side update semantics для redacted placeholders: application save/update paths и backup metadata update больше не persist-ят `__DOKPLOY_REDACTED_SECRET__`, а сохраняют текущие stored values. - Закрыл Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding и safer custom Git URL handling. - Закрепил schedule/deployment worker jobs signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection и explicit package exports for signed schedule/deployment helpers. - Улучшил monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits и responsive layout. - Добавил safe application env upsert: dry-run metadata, revision checks, secret-aware result summaries и conflict protection. - Обновил DB/schema/tooling surface для SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path и server package export switch scripts with stable final newline output. - После Codex Review restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into new forward-only migration `0175_guard_host_schedule_scope`. - Добавил и актуализировал regression coverage across auth, backup, deploy, Docker, Git providers, monitoring, schedule/deployment signing, permissions, redaction, safe paths, webhooks, package exports, migration placement и secret placeholder preservation. Зачем: - Для владельца Dokploy это снижает риск IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets, stale worker job execution и accidental secret corruption through redacted edit forms. - Для maintainer review это превращает большой набор security/UX gaps в проверяемые helpers, negative fixtures, focused tests и повторяемые package/build boundaries. - Для операторов Monitoring становится понятнее: видно, какие containers/images/volumes/processes используют ресурсы, а Docker Disk Usage раскрывается без потери контекста. - Для integrations safe env upsert дает менее рискованный путь точечно обновлять application environment variables без silent overwrite. - Для PR Dokploy#4708 review comments это закрывает runtime package-export gap, keeps already-applied migrations immutable, and preserves stored secrets when redacted placeholders return from edit forms. Риски: - PR commit остается большим и squashed; maintainer может попросить разделить security, monitoring и dependency/tooling parts на несколько PR. - Dependency/tooling refresh широкий, поэтому upstream CI после push остается финальным source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners, because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Backup metadata preservation now merges partial input metadata with existing metadata to avoid dropping stored secret sections; direct API clients that intended to clear omitted metadata must send explicit empty values. - Browser smoke был локальным unauthenticated boot/render smoke: login page, `/` HTTP 200 и screenshot; authenticated dashboard flows локально не проверялись. - Локальная среда использовала Node v24.18.0 при `.nvmrc` 24.4.0; engine range допускает 24.x, но это отличается от exact project recommendation. Проверки: - Команды и результаты: `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm --filter=dokploy exec vitest --config __test__/vitest.config.ts run __test__/git-provider/deploy-source-credential-access.test.ts __test__/backup/restore-security.test.ts --reporter=verbose` passed with 2 files / 72 tests; scoped Biome check on the 4 changed files passed; `git diff --check` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api and schedules; `CI=true corepack pnpm run server:build` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `git diff -- packages/server/package.json` was empty; `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm run test` passed with 152 files / 1337 tests / 1 skipped; independent Agent Flow reviewer.qa returned pass after the partial backup metadata merge follow-up. - Ограничения: upstream CI, maintainer review, authenticated browser flows, production deployment and a real DB migration fixture for `0175` were not run locally. What: - Strengthened authorization boundaries across router/service/WSS layers: service access, assigned-server access, organization scoping, permission checks, and safer deploy-source ownership checks. - Added reusable access helpers for Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets, and provider ownership. - Hardened command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization, and SSRF-aware URL checks. - Added secret redaction for API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context, and logs. - Preserved server-side update semantics for redacted placeholders: application save/update paths and backup metadata update no longer persist `__DOKPLOY_REDACTED_SECRET__`, and instead keep current stored values. - Closed Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding, and safer custom Git URL handling. - Bound schedule/deployment worker jobs with signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection, and explicit package exports for signed schedule/deployment helpers. - Improved monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits, and responsive layout. - Added safe application env upsert with dry-run metadata, revision checks, secret-aware result summaries, and conflict protection. - Updated DB/schema/tooling surface for SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path, and server package export switch scripts with stable final-newline output. - After Codex Review, restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into the new forward-only migration `0175_guard_host_schedule_scope`. - Added and updated regression coverage across auth, backup, deploy, Docker, Git providers, monitoring, schedule/deployment signing, permissions, redaction, safe paths, webhooks, package exports, migration placement, and secret placeholder preservation. Why: - For the Dokploy owner, this lowers the risk of IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets, stale worker job execution, and accidental secret corruption through redacted edit forms. - For maintainer review, this turns a large set of security and UX gaps into reviewable helpers, negative fixtures, focused tests, and repeatable package/build boundaries. - For operators, Monitoring becomes clearer: containers/images/volumes/processes show resource ownership, and Docker Disk Usage can expand without losing context. - For integrations, safe env upsert provides a lower-risk way to patch application environment variables without silent overwrite. - For PR Dokploy#4708 review comments, this closes the runtime package-export gap, keeps already-applied migrations immutable, and preserves stored secrets when redacted placeholders return from edit forms. Risks: - The PR commit remains large and squashed; the maintainer may ask to split security, monitoring, and dependency/tooling parts into separate PRs. - The dependency/tooling refresh is broad, so upstream CI after push remains the final source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Backup metadata preservation now merges partial input metadata with existing metadata to avoid dropping stored secret sections; direct API clients that intended to clear omitted metadata must send explicit empty values. - Browser smoke was a local unauthenticated boot/render smoke: login page, `/` HTTP 200, and screenshot; authenticated dashboard flows were not checked locally. - The local environment used Node v24.18.0 while `.nvmrc` is 24.4.0; the engine range accepts 24.x, but it differs from the exact project recommendation. Checks: - Commands and results: `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm --filter=dokploy exec vitest --config __test__/vitest.config.ts run __test__/git-provider/deploy-source-credential-access.test.ts __test__/backup/restore-security.test.ts --reporter=verbose` passed with 2 files / 72 tests; scoped Biome check on the 4 changed files passed; `git diff --check` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api and schedules; `CI=true corepack pnpm run server:build` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `git diff -- packages/server/package.json` was empty; `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm run test` passed with 152 files / 1337 tests / 1 skipped; independent Agent Flow reviewer.qa returned pass after the partial backup metadata merge follow-up. - Limitations: upstream CI, maintainer review, authenticated browser flows, production deployment, and a real DB migration fixture for `0175` were not run locally.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 814b7764ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилил authorization boundaries в router/service/WSS слоях: service access, assigned-server access, organization scoping, permission checks и safer deploy-source ownership checks. - Добавил переиспользуемые access helpers для Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets и provider ownership. - Усилил command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization и SSRF-aware URL checks. - Добавил secret redaction для API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context и logs. - Сохранил server-side update semantics для redacted placeholders: application save/update paths и backup metadata update больше не persist-ят `__DOKPLOY_REDACTED_SECRET__`, а сохраняют текущие stored values. - Закрыл Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding и safer custom Git URL handling. - Закрепил schedule/deployment worker jobs signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection и explicit package exports for signed schedule/deployment helpers. - Улучшил monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits и responsive layout. - Добавил safe application env upsert: dry-run metadata, revision checks, secret-aware result summaries и conflict protection. - Обновил DB/schema/tooling surface для SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path и server package export switch scripts with stable final newline output. - После Codex Review restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into new forward-only migration `0175_guard_host_schedule_scope`. - Закрыл свежие Codex Review P1/P2: trusted monitoring-agent HTTP fetch теперь проходит через explicit private-network allow, GitLab deploy webhooks больше не используют OAuth client secret как `X-Gitlab-Token`, а local host container stats фильтруются через service-level monitoring access. - Добавил Docker stats label enrichment и regression coverage для GitLab webhook auth, monitoring fetch options, container stats access filtering и Docker label parsing. Зачем: - Для владельца Dokploy это снижает риск IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets, stale worker job execution и accidental secret corruption through redacted edit forms. - Для maintainer review это превращает большой набор security/UX gaps в проверяемые helpers, negative fixtures, focused tests и повторяемые package/build boundaries. - Для операторов Monitoring становится понятнее: видно, какие containers/images/volumes/processes используют ресурсы, а Docker Disk Usage раскрывается без потери контекста. - Для integrations safe env upsert дает менее рискованный путь точечно обновлять application environment variables без silent overwrite. - Для PR Dokploy#4708 review comments это закрывает runtime package-export gap, keeps already-applied migrations immutable, preserves stored secrets when redacted placeholders return from edit forms, keeps cloud monitoring metrics working with the trusted HTTP agent и не требует раскрывать GitLab OAuth client secret в webhook settings. Риски: - PR commit остается большим и squashed; maintainer может попросить разделить security, monitoring и dependency/tooling parts на несколько PR. - Dependency/tooling refresh широкий, поэтому upstream CI после push остается финальным source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners, because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Backup metadata preservation now merges partial input metadata with existing metadata to avoid dropping stored secret sections; direct API clients that intended to clear omitted metadata must send explicit empty values. - GitLab dedicated webhook token storage/UI is not added in this review-fix; existing GitLab refresh-token deploy webhooks remain compatible and are protected by the refresh token plus branch/watch-path checks, while `X-Gitlab-Token` is verified only if a separate provider value exists. - Container stats filtering depends on Docker labels or exact Docker service names being present; broad prefix matching is intentionally not used to avoid leaking similarly named services. - Browser smoke был локальным unauthenticated boot/render smoke: login page, `/` HTTP 200 и screenshot; authenticated dashboard flows локально не проверялись. - Локальная среда использовала Node v24.18.0 при `.nvmrc` 24.4.0; engine range допускает 24.x, но это отличается от exact project recommendation. Проверки: - Команды и результаты: focused Vitest for server metrics, container stats access, Docker label parsing, user container metrics, application refresh-token webhooks and compose refresh-token webhooks passed with 6 files / 44 tests; `git diff --check` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api and schedules; `CI=true corepack pnpm run server:build` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `git diff -- packages/server/package.json` was empty; `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm run test` passed with 154 files / 1344 tests / 1 skipped; mandatory independent Agent Flow QA Verifier returned pass. - Ограничения: upstream CI, maintainer review, authenticated browser flows, production deployment, live Docker Swarm/Compose runtime validation, and a real DB migration fixture for `0175` were not run locally. What: - Strengthened authorization boundaries across router/service/WSS layers: service access, assigned-server access, organization scoping, permission checks, and safer deploy-source ownership checks. - Added reusable access helpers for Docker/local server, monitoring targets, deploy-source credentials, destination binding, backup/schedule targets, and provider ownership. - Hardened command/path safety: shell argument quoting, Git clone/build commands, custom compose command validation, Dockerfile/build command boundaries, bind/restore path normalization, and SSRF-aware URL checks. - Added secret redaction for API responses, nested service reads, notification/error paths, Git providers, SSH keys, registry data, deployment/rollback context, and logs. - Preserved server-side update semantics for redacted placeholders: application save/update paths and backup metadata update no longer persist `__DOKPLOY_REDACTED_SECRET__`, and instead keep current stored values. - Closed Git provider/webhook gaps: GitHub App OAuth state signing, provider read/update access checks, webhook payload binding, and safer custom Git URL handling. - Bound schedule/deployment worker jobs with signed metadata: scoped queue payloads, immutable job claims, stale/foreign job rejection, and explicit package exports for signed schedule/deployment helpers. - Improved monitoring UX: resource breakdown by container, process parsing, swap reporting, Docker Disk Usage details, image/container/volume context, visible detail limits, and responsive layout. - Added safe application env upsert with dry-run metadata, revision checks, secret-aware result summaries, and conflict protection. - Updated DB/schema/tooling surface for SSO/Gitea scoping, refreshed dependencies, TypeScript/Next build path, and server package export switch scripts with stable final-newline output. - After Codex Review, restored historical migration `0169` to the upstream form and moved host-level schedule cleanup into the new forward-only migration `0175_guard_host_schedule_scope`. - Closed the latest Codex Review P1/P2 comments: trusted monitoring-agent HTTP fetch now uses an explicit private-network allow, GitLab deploy webhooks no longer use the OAuth client secret as `X-Gitlab-Token`, and local host container stats are filtered through service-level monitoring access. - Added Docker stats label enrichment and regression coverage for GitLab webhook auth, monitoring fetch options, container stats access filtering, and Docker label parsing. Why: - For the Dokploy owner, this lowers the risk of IDOR, cross-server access, SSRF, command injection, unsafe path access, leaked secrets, stale worker job execution, and accidental secret corruption through redacted edit forms. - For maintainer review, this turns a large set of security and UX gaps into reviewable helpers, negative fixtures, focused tests, and repeatable package/build boundaries. - For operators, Monitoring becomes clearer: containers/images/volumes/processes show resource ownership, and Docker Disk Usage can expand without losing context. - For integrations, safe env upsert provides a lower-risk way to patch application environment variables without silent overwrite. - For PR Dokploy#4708 review comments, this closes the runtime package-export gap, keeps already-applied migrations immutable, preserves stored secrets when redacted placeholders return from edit forms, keeps cloud monitoring metrics working with the trusted HTTP agent, and avoids exposing the GitLab OAuth client secret in webhook settings. Risks: - The PR commit remains large and squashed; the maintainer may ask to split security, monitoring, and dependency/tooling parts into separate PRs. - The dependency/tooling refresh is broad, so upstream CI after push remains the final source of truth. - Migration `0175_guard_host_schedule_scope` fail-closes host-level schedules for organizations touched by ambiguous multi-org owners because exact original `schedule.userId` provenance is no longer recoverable after historical `0169`; affected users may need to re-enable reviewed schedules. - Backup metadata preservation now merges partial input metadata with existing metadata to avoid dropping stored secret sections; direct API clients that intended to clear omitted metadata must send explicit empty values. - GitLab dedicated webhook token storage/UI is not added in this review fix; existing GitLab refresh-token deploy webhooks remain compatible and are protected by the refresh token plus branch/watch-path checks, while `X-Gitlab-Token` is verified only if a separate provider value exists. - Container stats filtering depends on Docker labels or exact Docker service names being present; broad prefix matching is intentionally not used to avoid leaking similarly named services. - Browser smoke was a local unauthenticated boot/render smoke: login page, `/` HTTP 200, and screenshot; authenticated dashboard flows were not checked locally. - The local environment used Node v24.18.0 while `.nvmrc` is 24.4.0; the engine range accepts 24.x, but it differs from the exact project recommendation. Checks: - Commands and results: focused Vitest for server metrics, container stats access, Docker label parsing, user container metrics, application refresh-token webhooks, and compose refresh-token webhooks passed with 6 files / 44 tests; `git diff --check` passed; `CI=true corepack pnpm run format-and-lint` passed with existing 37 warnings and 26 infos; `CI=true corepack pnpm run typecheck` passed for server, dokploy, api, and schedules; `CI=true corepack pnpm run server:build` passed; `corepack pnpm run server:script` restored source exports; `CI=true corepack pnpm run build` passed with build-only `BETTER_AUTH_SECRET` warnings; `corepack pnpm run server:script` restored source exports after build; `git diff -- packages/server/package.json` was empty; `PATH="/Applications/Docker.app/Contents/Resources/bin:$PATH" CI=true corepack pnpm run test` passed with 154 files / 1344 tests / 1 skipped; mandatory independent Agent Flow QA Verifier returned pass. - Limitations: upstream CI, maintainer review, authenticated browser flows, production deployment, live Docker Swarm/Compose runtime validation, and a real DB migration fixture for `0175` were not run locally.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fe63e8373
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Добавлен GitLab deploy webhook secret как отдельный nullable DB/API/UI field; GitLab refresh-token deploy теперь fail-closed без реального webhook secret, а non-GitHub provider fallback сохранен. - Обновлен Monitoring UI: Docker Disk Usage получил подробные inspect details, лимит видимых строк, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - PR Dokploy#4708 перебазирован на upstream/canary d87229c; конфликт с upstream ssh-keyscan и ECR registry username casing разрешен, registry regression test обновлен под сохранение casing. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks для PR не репортятся автоматически, поэтому локальные проверки и независимая QA остаются основным доказательством. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; pnpm format-and-lint passed with existing warnings only; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; direct apps/dokploy build-next passed after one transient Next/Webpack WasmHash failure; repeated pnpm build passed; full pnpm test passed 156 files / 1360 tests / 1 skipped; git diff --cached --check passed. - Ограничения: authenticated browser flows, production deploy, real upgrade on VPS, and GitHub-hosted checks were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Added the GitLab deploy webhook secret as a separate nullable DB/API/UI field; GitLab refresh-token deploy now fails closed without a real webhook secret while preserving the non-GitHub provider fallback. - Updated the Monitoring UI: Docker Disk Usage now has detailed inspect context, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - Rebasing PR Dokploy#4708 onto upstream/canary d87229c resolved the upstream ssh-keyscan and ECR registry username casing conflicts, and the registry regression test now expects preserved username casing. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks still do not report automatically for the PR, so local checks and independent QA remain the primary evidence. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; pnpm format-and-lint passed with existing warnings only; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; direct apps/dokploy build-next passed after one transient Next/Webpack WasmHash failure; repeated pnpm build passed; full pnpm test passed 156 files / 1360 tests / 1 skipped; git diff --cached --check passed. - Limitations: authenticated browser flows, production deploy, real VPS upgrade, and GitHub-hosted checks were not run in this commit.
|
Final update for
Verification for
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5628cc4d65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - `server.getContainerProcesses` теперь разрешает Docker process list только через уже доступные local monitoring stats и не вызывает `docker top` для произвольного container id/name. - Добавлен GitLab deploy webhook secret как отдельный nullable DB/API/UI field; GitLab refresh-token deploy теперь fail-closed без реального webhook secret, а non-GitHub provider fallback сохранен. - Обновлен Monitoring UI: Docker Disk Usage получил подробные inspect details, лимит видимых строк, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - PR Dokploy#4708 перебазирован на upstream/canary d87229c; конфликт с upstream ssh-keyscan и ECR registry username casing разрешен, registry regression test обновлен под сохранение casing. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы пользователь с `monitoring: read` не мог получать `docker top` output для чужого или непривязанного контейнера. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks для PR не репортятся автоматически, поэтому локальные проверки и независимая QA остаются основным доказательством. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; scoped Biome for the new P1 files passed; pnpm format-and-lint passed with 3 existing warnings; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; initial pnpm build without DB env failed on missing POSTGRES_PASSWORD, then POSTGRES_PASSWORD=dokploy pnpm build passed and server:script restored source exports; full pnpm test passed 156 files / 1363 tests / 1 skipped; git diff --cached --check passed; mandatory independent Agent Flow QA passed. - Ограничения: authenticated browser flows, production deploy, real upgrade on VPS, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - `server.getContainerProcesses` now resolves Docker process lists only through already accessible local monitoring stats and does not call `docker top` for arbitrary container ids or names. - Added the GitLab deploy webhook secret as a separate nullable DB/API/UI field; GitLab refresh-token deploy now fails closed without a real webhook secret while preserving the non-GitHub provider fallback. - Updated the Monitoring UI: Docker Disk Usage now has detailed inspect context, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - Rebasing PR Dokploy#4708 onto upstream/canary d87229c resolved the upstream ssh-keyscan and ECR registry username casing conflicts, and the registry regression test now expects preserved username casing. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So a user with `monitoring: read` cannot receive `docker top` output for a foreign or unlinked container. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks still do not report automatically for the PR, so local checks and independent QA remain the primary evidence. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; scoped Biome for the new P1 files passed; pnpm format-and-lint passed with 3 existing warnings; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; initial pnpm build without DB env failed on missing POSTGRES_PASSWORD, then POSTGRES_PASSWORD=dokploy pnpm build passed and server:script restored source exports; full pnpm test passed 156 files / 1363 tests / 1 skipped; git diff --cached --check passed; mandatory independent Agent Flow QA passed. - Limitations: authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit.
|
Final update for
Verification for
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ce5e07d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - `server.getContainerProcesses` теперь разрешает Docker process list только через уже доступные local monitoring stats и не вызывает `docker top` для произвольного container id/name. - GitHub application/compose refresh-token deploy webhooks теперь проверяют подпись по raw request body, а не по `JSON.stringify(req.body)`, сохраняя parsed body для существующей deploy-логики. - Schedule queue signing получил совместимый upgrade path: при отсутствии `SCHEDULES_SIGNING_KEY` ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с явной setup-подсказкой. - Добавлен GitLab deploy webhook secret как отдельный nullable DB/API/UI field; GitLab refresh-token deploy теперь fail-closed без реального webhook secret, а non-GitHub provider fallback сохранен. - Обновлен Monitoring UI: Docker Disk Usage получил подробные inspect details, лимит видимых строк, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - PR Dokploy#4708 перебазирован на upstream/canary d87229c; конфликт с upstream ssh-keyscan и ECR registry username casing разрешен, registry regression test обновлен под сохранение casing. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы пользователь с `monitoring: read` не мог получать `docker top` output для чужого или непривязанного контейнера. - Чтобы существующие GitHub auto-deploy webhooks и schedule-management flows не ломались после обновления из-за raw-body verification и нового schedule signing key. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks для PR не репортятся автоматически, поэтому локальные проверки и независимая QA остаются основным доказательством. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; scoped Biome for each Codex P1 follow-up passed; pnpm format-and-lint passed with 3 existing warnings; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; initial pnpm build without DB env failed on missing POSTGRES_PASSWORD, then POSTGRES_PASSWORD=dokploy pnpm build passed and server:script restored source exports; full pnpm test passed 156 files / 1365 tests / 1 skipped; git diff --cached --check passed; mandatory independent Agent Flow QA passed for each follow-up, including Franklin pass for the raw-body/schedule-key fix. - Ограничения: authenticated browser flows, production deploy, real upgrade on VPS, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - `server.getContainerProcesses` now resolves Docker process lists only through already accessible local monitoring stats and does not call `docker top` for arbitrary container ids or names. - GitHub application/compose refresh-token deploy webhooks now verify signatures against the raw request body instead of `JSON.stringify(req.body)` while preserving the parsed body for the existing deploy logic. - Schedule queue signing now has a compatible upgrade path: when `SCHEDULES_SIGNING_KEY` is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with explicit setup guidance. - Added the GitLab deploy webhook secret as a separate nullable DB/API/UI field; GitLab refresh-token deploy now fails closed without a real webhook secret while preserving the non-GitHub provider fallback. - Updated the Monitoring UI: Docker Disk Usage now has detailed inspect context, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - Rebasing PR Dokploy#4708 onto upstream/canary d87229c resolved the upstream ssh-keyscan and ECR registry username casing conflicts, and the registry regression test now expects preserved username casing. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So a user with `monitoring: read` cannot receive `docker top` output for a foreign or unlinked container. - So existing GitHub auto-deploy webhooks and schedule-management flows do not break after upgrade because of raw-body verification and the new schedule signing key. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks still do not report automatically for the PR, so local checks and independent QA remain the primary evidence. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; scoped Biome for each Codex P1 follow-up passed; pnpm format-and-lint passed with 3 existing warnings; pnpm typecheck passed; pnpm server:build passed and pnpm server:script restored @dokploy/server source exports; initial pnpm build without DB env failed on missing POSTGRES_PASSWORD, then POSTGRES_PASSWORD=dokploy pnpm build passed and server:script restored source exports; full pnpm test passed 156 files / 1365 tests / 1 skipped; git diff --cached --check passed; mandatory independent Agent Flow QA passed for each follow-up, including Franklin pass for the raw-body/schedule-key fix. - Limitations: authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit.
|
Final update for
Verification for
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b0eb5ffb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Docker/monitoring endpoints теперь фильтруют container stats/process lists через service-aware access checks; `server.getContainerProcesses` не вызывает `docker top` для чужого или непривязанного container id/name. - Локальные app-name Docker routes теперь сохраняют service-scoped доступ: service-read listing идет через `service:read`, Docker app-name routes через `docker:read`, а remote `serverId` paths остаются за owner/admin Docker host gate. - Cancel deployment API verification теперь сохраняет `requireActiveServer: false`, поэтому stuck queued/running deployments можно отменять даже если assigned server уже inactive. - GitHub application/compose refresh-token deploy webhooks проверяют подпись по raw request body, сохраняя parsed body для существующей deploy-логики. - Schedule queue signing получил совместимый upgrade path: при отсутствии `SCHEDULES_SIGNING_KEY` ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с setup-подсказкой. - GitLab deploy webhook secret вынесен в отдельный nullable DB/API/UI field; OAuth `gitlab.secret` больше не принимается как webhook token, а non-GitHub provider fallback сохранен. - Application и backup update paths сохраняют реальные секреты при сохранении redacted placeholders из edit forms. - Обновлен Monitoring UI: Docker Disk Usage получил inspect details, visible row limits, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - `getLocalServerIp` снова использует literal escaped dots в RFC1918 `grep -E` pattern. - PR Dokploy#4708 перебазирован на upstream/canary d87229c; конфликт с upstream ssh-keyscan и ECR registry username casing разрешен. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы service-scoped пользователи не теряли доступ к своим local container/log views и одновременно не получали Docker host access к чужим контейнерам. - Чтобы пользователи могли отменять deployment jobs после того, как целевой server стал inactive. - Чтобы существующие GitHub/GitLab deploy webhooks, schedule-management flows и edit/save формы с секретами не ломались после обновления. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks для PR не репортятся автоматически, поэтому локальные проверки и независимая QA остаются основным доказательством. - Full local `pnpm test` после последнего P2/P3 follow-up не прошел только на `__test__/deploy/application.real.test.ts` real deploy/Nixpacks сценариях, хотя этот же real deploy файл проходил изолированно раньше в run. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; scoped Biome for latest 2 changed files passed; latest `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed for each follow-up, including Pasteur pass for this cancel inactive server follow-up. - Ограничения: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Docker/monitoring endpoints now filter container stats and process lists through service-aware access checks; `server.getContainerProcesses` does not call `docker top` for foreign or unlinked container ids/names. - Local app-name Docker routes now preserve service-scoped access: service-read listing uses `service:read`, Docker app-name routes use `docker:read`, and remote `serverId` paths stay behind the owner/admin Docker host gate. - Cancel deployment API verification now preserves `requireActiveServer: false`, so stuck queued/running deployments can be cancelled even after the assigned server becomes inactive. - GitHub application/compose refresh-token deploy webhooks verify signatures against the raw request body while preserving the parsed body for the existing deploy logic. - Schedule queue signing now has a compatible upgrade path: when `SCHEDULES_SIGNING_KEY` is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with setup guidance. - The GitLab deploy webhook secret is a separate nullable DB/API/UI field; OAuth `gitlab.secret` is no longer accepted as a webhook token, while the non-GitHub provider fallback is preserved. - Application and backup update paths preserve real secrets when edit forms submit redacted placeholders. - Updated the Monitoring UI: Docker Disk Usage now has inspect details, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - `getLocalServerIp` again uses literal escaped dots in the RFC1918 `grep -E` pattern. - Rebasing PR Dokploy#4708 onto upstream/canary d87229c resolved the upstream ssh-keyscan and ECR registry username casing conflicts. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So service-scoped users keep access to their own local container/log views without receiving Docker host access to foreign containers. - So users can cancel deployment jobs after the target server has become inactive. - So existing GitHub/GitLab deploy webhooks, schedule-management flows, and secret-bearing edit/save forms do not break after upgrade. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks still do not report automatically for the PR, so local checks and independent QA remain the primary evidence. - Full local `pnpm test` after the latest P2/P3 follow-up failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks scenarios, although the same real deploy file passed in isolation earlier in the run. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; scoped Biome for latest 2 changed files passed; latest `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed for each follow-up, including Pasteur pass for this cancel inactive server follow-up. - Limitations: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit.
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Docker/monitoring endpoints теперь фильтруют container stats/process lists через service-aware access checks; `server.getContainerProcesses` не вызывает `docker top` для чужого или непривязанного container id/name. - Локальные app-name Docker routes теперь сохраняют service-scoped доступ: service-read listing идет через `service:read`, Docker app-name routes через `docker:read`, а remote `serverId` paths остаются за owner/admin Docker host gate. - Cancel deployment API verification теперь сохраняет `requireActiveServer: false`, поэтому stuck queued/running deployments можно отменять даже если assigned server уже inactive. - GitHub application/compose refresh-token deploy webhooks проверяют подпись по raw request body, сохраняя parsed body для существующей deploy-логики. - Schedule queue signing получил совместимый upgrade path: при отсутствии `SCHEDULES_SIGNING_KEY` ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с setup-подсказкой. - GitLab deploy webhook secret вынесен в отдельный nullable DB/API/UI field; OAuth `gitlab.secret` больше не принимается как webhook token, а non-GitHub provider fallback сохранен. - Application и backup update paths сохраняют реальные секреты при сохранении redacted placeholders из edit forms. - Обновлен Monitoring UI: Docker Disk Usage получил inspect details, visible row limits, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - `getLocalServerIp` снова использует literal escaped dots в RFC1918 `grep -E` pattern. - PR Dokploy#4708 перебазирован на upstream/canary 91abc93; конфликты с upstream ssh-keyscan, ECR registry username casing и GitHub webhook owner matching разрешены без отката upstream behavior. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы service-scoped пользователи не теряли доступ к своим local container/log views и одновременно не получали Docker host access к чужим контейнерам. - Чтобы пользователи могли отменять deployment jobs после того, как целевой server стал inactive. - Чтобы существующие GitHub/GitLab deploy webhooks, schedule-management flows и edit/save формы с секретами не ломались после обновления. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks для PR не репортятся автоматически, поэтому локальные проверки и независимая QA остаются основным доказательством. - Full local `pnpm test` после последнего P2/P3 follow-up не прошел только на `__test__/deploy/application.real.test.ts` real deploy/Nixpacks сценариях, хотя этот же real deploy файл проходил изолированно раньше в run. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; scoped Biome for post-rebase 4 files passed; latest `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed for each follow-up, including Pasteur pass for this cancel inactive server follow-up. - Ограничения: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Docker/monitoring endpoints now filter container stats and process lists through service-aware access checks; `server.getContainerProcesses` does not call `docker top` for foreign or unlinked container ids/names. - Local app-name Docker routes now preserve service-scoped access: service-read listing uses `service:read`, Docker app-name routes use `docker:read`, and remote `serverId` paths stay behind the owner/admin Docker host gate. - Cancel deployment API verification now preserves `requireActiveServer: false`, so stuck queued/running deployments can be cancelled even after the assigned server becomes inactive. - GitHub application/compose refresh-token deploy webhooks verify signatures against the raw request body while preserving the parsed body for the existing deploy logic. - Schedule queue signing now has a compatible upgrade path: when `SCHEDULES_SIGNING_KEY` is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with setup guidance. - The GitLab deploy webhook secret is a separate nullable DB/API/UI field; OAuth `gitlab.secret` is no longer accepted as a webhook token, while the non-GitHub provider fallback is preserved. - Application and backup update paths preserve real secrets when edit forms submit redacted placeholders. - Updated the Monitoring UI: Docker Disk Usage now has inspect details, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - `getLocalServerIp` again uses literal escaped dots in the RFC1918 `grep -E` pattern. - Rebasing PR Dokploy#4708 onto upstream/canary 91abc93 resolved the upstream ssh-keyscan, ECR registry username casing, and GitHub webhook owner matching conflicts without reverting upstream behavior. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So service-scoped users keep access to their own local container/log views without receiving Docker host access to foreign containers. - So users can cancel deployment jobs after the target server has become inactive. - So existing GitHub/GitLab deploy webhooks, schedule-management flows, and secret-bearing edit/save forms do not break after upgrade. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks still do not report automatically for the PR, so local checks and independent QA remain the primary evidence. - Full local `pnpm test` after the latest P2/P3 follow-up failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks scenarios, although the same real deploy file passed in isolation earlier in the run. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; scoped Biome for post-rebase 4 files passed; latest `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed for each follow-up, including Pasteur pass for this cancel inactive server follow-up. - Limitations: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit.
|
Final update for
Fresh post-rebase verification passed:
GitHub checks are now reporting and running for the updated head. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da16761634
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Docker/monitoring endpoints теперь фильтруют container stats/process lists через service-aware access checks; `server.getContainerProcesses` не вызывает `docker top` для чужого или непривязанного container id/name. - Локальные app-name Docker routes теперь сохраняют service-scoped доступ: service-read listing идет через `service:read`, Docker app-name routes через `docker:read`, а remote `serverId` paths остаются за owner/admin Docker host gate. - Cancel deployment API verification теперь сохраняет `requireActiveServer: false`, поэтому stuck queued/running deployments можно отменять даже если assigned server уже inactive. - GitHub application/compose refresh-token deploy webhooks проверяют подпись по raw request body, сохраняя parsed body для существующей deploy-логики. - Schedule queue signing получил совместимый upgrade path: при отсутствии `SCHEDULES_SIGNING_KEY` ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с setup-подсказкой. - GitLab deploy webhook secret вынесен в отдельный nullable DB/API/UI field; OAuth `gitlab.secret` больше не принимается как webhook token, а non-GitHub provider fallback сохранен. - Application и backup update paths сохраняют реальные секреты при сохранении redacted placeholders из edit forms. - Обновлен Monitoring UI: Docker Disk Usage получил inspect details, visible row limits, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - `getLocalServerIp` снова использует literal escaped dots в RFC1918 `grep -E` pattern. - PR Dokploy#4708 перебазирован на upstream/canary 91abc93; конфликты с upstream ssh-keyscan, ECR registry username casing и GitHub webhook owner matching разрешены без отката upstream behavior. - Next production build imports больше не требуют runtime Postgres password во время `phase-production-build`, но обычный production runtime без password/file продолжает fail-closed. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы service-scoped пользователи не теряли доступ к своим local container/log views и одновременно не получали Docker host access к чужим контейнерам. - Чтобы пользователи могли отменять deployment jobs после того, как целевой server стал inactive. - Чтобы существующие GitHub/GitLab deploy webhooks, schedule-management flows и edit/save формы с секретами не ломались после обновления. - Чтобы GitHub Actions `pnpm build` не требовал runtime DB secret при статическом Next build import, сохраняя runtime security boundary. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks теперь репортятся, но после CI build fix их нужно дождаться на новом pushed head. - Full local `pnpm test` после последнего P2/P3 follow-up не прошел только на `__test__/deploy/application.real.test.ts` real deploy/Nixpacks сценариях, хотя этот же real deploy файл проходил изолированно раньше в run. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; scoped Biome for post-rebase/CI-fix 6 files passed; latest `git diff --check` passed; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; local no-secret `pnpm build` progressed past the original CI DB-password error but hit a local Node 24.18.0 Next/Webpack `WasmHash` failure; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including Pasteur pass for cancel inactive server and Rawls pass-with-risks for CI build fix due QA not rerunning full build/typecheck/server:build itself. - Ограничения: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Docker/monitoring endpoints now filter container stats and process lists through service-aware access checks; `server.getContainerProcesses` does not call `docker top` for foreign or unlinked container ids/names. - Local app-name Docker routes now preserve service-scoped access: service-read listing uses `service:read`, Docker app-name routes use `docker:read`, and remote `serverId` paths stay behind the owner/admin Docker host gate. - Cancel deployment API verification now preserves `requireActiveServer: false`, so stuck queued/running deployments can be cancelled even after the assigned server becomes inactive. - GitHub application/compose refresh-token deploy webhooks verify signatures against the raw request body while preserving the parsed body for the existing deploy logic. - Schedule queue signing now has a compatible upgrade path: when `SCHEDULES_SIGNING_KEY` is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with setup guidance. - The GitLab deploy webhook secret is a separate nullable DB/API/UI field; OAuth `gitlab.secret` is no longer accepted as a webhook token, while the non-GitHub provider fallback is preserved. - Application and backup update paths preserve real secrets when edit forms submit redacted placeholders. - Updated the Monitoring UI: Docker Disk Usage now has inspect details, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - `getLocalServerIp` again uses literal escaped dots in the RFC1918 `grep -E` pattern. - Rebasing PR Dokploy#4708 onto upstream/canary 91abc93 resolved the upstream ssh-keyscan, ECR registry username casing, and GitHub webhook owner matching conflicts without reverting upstream behavior. - Next production build imports no longer require a runtime Postgres password during `phase-production-build`, while ordinary production runtime without a password/file still fails closed. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So service-scoped users keep access to their own local container/log views without receiving Docker host access to foreign containers. - So users can cancel deployment jobs after the target server has become inactive. - So existing GitHub/GitLab deploy webhooks, schedule-management flows, and secret-bearing edit/save forms do not break after upgrade. - So GitHub Actions `pnpm build` does not require a runtime DB secret for static Next build imports while keeping the runtime security boundary. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks now report, but they need to rerun on the new pushed head after the CI build fix. - Full local `pnpm test` after the latest P2/P3 follow-up failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks scenarios, although the same real deploy file passed in isolation earlier in the run. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; scoped Biome for post-rebase/CI-fix 6 files passed; latest `git diff --check` passed; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; local no-secret `pnpm build` progressed past the original CI DB-password error but hit a local Node 24.18.0 Next/Webpack `WasmHash` failure; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including Pasteur pass for cancel inactive server and Rawls pass-with-risks for CI build fix because QA did not rerun full build/typecheck/server:build itself. - Limitations: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks, and a separate Codex Security scan were not run in this commit.
|
Final update for
Verification for
GitHub checks are rerunning for the new pushed head now. @codex review |
|
@Siumauricio Hey, bro, let's update all the libraries to the latest versions. All the vulnerabilities will be gone. Refactor them if needed. I really want you to release this—many of the libraries are outdated and vulnerable, and I want to help you improve the dokploy product. I need your help and expertise in this matter. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 172102a012
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Docker/monitoring endpoints теперь фильтруют container stats/process lists через service-aware access checks; `server.getContainerProcesses` не вызывает `docker top` для чужого или непривязанного container id/name. - Локальные app-name Docker routes теперь сохраняют service-scoped доступ: service-read listing идет через `service:read`, Docker app-name routes через `docker:read`, а remote `serverId` paths остаются за owner/admin Docker host gate. - Cancel deployment API verification теперь сохраняет `requireActiveServer: false`, поэтому stuck queued/running deployments можно отменять даже если assigned server уже inactive. - GitHub application/compose refresh-token deploy webhooks проверяют подпись по raw request body, сохраняя parsed body для существующей deploy-логики. - Schedule queue signing получил совместимый upgrade path: при отсутствии `SCHEDULES_SIGNING_KEY` ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с setup-подсказкой. - GitLab deploy webhook secret вынесен в отдельный nullable DB/API/UI field; OAuth `gitlab.secret` больше не принимается как webhook token, а non-GitHub provider fallback сохранен. - Application и backup update paths сохраняют реальные секреты при сохранении redacted placeholders из edit forms. - Обновлен Monitoring UI: Docker Disk Usage получил inspect details, visible row limits, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - `getLocalServerIp` снова использует literal escaped dots в RFC1918 `grep -E` pattern. - PR Dokploy#4708 перебазирован на upstream/canary 91abc93; конфликты с upstream ssh-keyscan, ECR registry username casing и GitHub webhook owner matching разрешены без отката upstream behavior. - Next production build imports больше не требуют runtime Postgres password во время `phase-production-build`, но обычный production runtime без password/file продолжает fail-closed. - Signed-job export-map regression теперь принимает оба допустимых состояния package exports: source mode после `server:script` и dist mode после `server:build`, чтобы `pnpm test` проходил в CI-порядке `server:build -> test`. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы service-scoped пользователи не теряли доступ к своим local container/log views и одновременно не получали Docker host access к чужим контейнерам. - Чтобы пользователи могли отменять deployment jobs после того, как целевой server стал inactive. - Чтобы существующие GitHub/GitLab deploy webhooks, schedule-management flows и edit/save формы с секретами не ломались после обновления. - Чтобы GitHub Actions `pnpm build` не требовал runtime DB secret при статическом Next build import, сохраняя runtime security boundary. - Чтобы GitHub Actions `pnpm test` не падал после production export switch, сохраняя проверку public signed-job subpath. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks нужно дождаться на новом pushed head после CI test export-map follow-up. - Full local `pnpm test` после последнего P2/P3 follow-up не прошел только на `__test__/deploy/application.real.test.ts` real deploy/Nixpacks сценариях, хотя этот же real deploy файл проходил изолированно раньше в run. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; CI test export-map focused Vitest passed 1 file / 11 tests in source-export mode and passed again after `pnpm server:build` in dist-export mode; scoped Biome for post-rebase/CI-fix/export-map files passed; latest `git diff --check` passed; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; local no-secret `pnpm build` progressed past the original CI DB-password error but hit a local Node 24.18.0 Next/Webpack `WasmHash` failure; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including Pasteur pass for cancel inactive server, Rawls pass-with-risks for CI build fix, and Linnaeus pass for CI test export-map fix. - Ограничения: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks on this amended head, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Docker/monitoring endpoints now filter container stats and process lists through service-aware access checks; `server.getContainerProcesses` does not call `docker top` for foreign or unlinked container ids/names. - Local app-name Docker routes now preserve service-scoped access: service-read listing uses `service:read`, Docker app-name routes use `docker:read`, and remote `serverId` paths stay behind the owner/admin Docker host gate. - Cancel deployment API verification now preserves `requireActiveServer: false`, so stuck queued/running deployments can be cancelled even after the assigned server becomes inactive. - GitHub application/compose refresh-token deploy webhooks verify signatures against the raw request body while preserving the parsed body for the existing deploy logic. - Schedule queue signing now has a compatible upgrade path: when `SCHEDULES_SIGNING_KEY` is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with setup guidance. - The GitLab deploy webhook secret is a separate nullable DB/API/UI field; OAuth `gitlab.secret` is no longer accepted as a webhook token, while the non-GitHub provider fallback is preserved. - Application and backup update paths preserve real secrets when edit forms submit redacted placeholders. - Updated the Monitoring UI: Docker Disk Usage now has inspect details, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - `getLocalServerIp` again uses literal escaped dots in the RFC1918 `grep -E` pattern. - Rebasing PR Dokploy#4708 onto upstream/canary 91abc93 resolved the upstream ssh-keyscan, ECR registry username casing, and GitHub webhook owner matching conflicts without reverting upstream behavior. - Next production build imports no longer require a runtime Postgres password during `phase-production-build`, while ordinary production runtime without a password/file still fails closed. - The signed-job export-map regression now accepts both valid package export states: source mode after `server:script` and dist mode after `server:build`, so `pnpm test` passes in the CI order `server:build -> test`. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So service-scoped users keep access to their own local container/log views without receiving Docker host access to foreign containers. - So users can cancel deployment jobs after the target server has become inactive. - So existing GitHub/GitLab deploy webhooks, schedule-management flows, and secret-bearing edit/save forms do not break after upgrade. - So GitHub Actions `pnpm build` does not require a runtime DB secret for static Next build imports while keeping the runtime security boundary. - So GitHub Actions `pnpm test` does not fail after the production export switch while still checking the public signed-job subpath. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks need to rerun on the new pushed head after the CI test export-map follow-up. - Full local `pnpm test` after the latest P2/P3 follow-up failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks scenarios, although the same real deploy file passed in isolation earlier in the run. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; latest cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; CI test export-map focused Vitest passed 1 file / 11 tests in source-export mode and passed again after `pnpm server:build` in dist-export mode; scoped Biome for post-rebase/CI-fix/export-map files passed; latest `git diff --check` passed; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed; local no-secret `pnpm build` progressed past the original CI DB-password error but hit a local Node 24.18.0 Next/Webpack `WasmHash` failure; `pnpm --filter @dokploy/api typecheck` passed; `pnpm --filter @dokploy/api build` passed; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including Pasteur pass for cancel inactive server, Rawls pass-with-risks for CI build fix, and Linnaeus pass for CI test export-map fix. - Limitations: full local `pnpm test` with Docker Desktop in PATH was retried in parallel and serial modes after the P2/P3 Docker route/IP follow-up and failed only in `__test__/deploy/application.real.test.ts`; authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks on this amended head, and a separate Codex Security scan were not run in this commit.
|
Final update for
Verification for
GitHub checks are rerunning for the new pushed head now. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0862208d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Усилен server-side security boundary для API/router/service путей, Docker/monitoring доступа, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction и secret preservation. - Docker/monitoring endpoints теперь фильтруют container stats/process lists через service-aware access checks; `server.getContainerProcesses` не вызывает `docker top` для чужого или непривязанного container id/name. - Локальные Docker container actions без `serverId` теперь используют service-scoped container access без owner/admin Docker host gate; remote `serverId` paths остаются за owner/admin Docker host gate. - Локальные app-name Docker routes теперь сохраняют service-scoped доступ: service-read listing идет через `service:read`, Docker app-name routes через `docker:read`, а remote `serverId` paths остаются за owner/admin Docker host gate. - Cancel deployment API verification теперь сохраняет `requireActiveServer: false`, поэтому stuck queued/running deployments можно отменять даже если assigned server уже inactive. - GitHub application/compose refresh-token deploy webhooks проверяют подпись по raw request body, сохраняя parsed body для существующей deploy-логики. - Schedule и deployment queue signing получили совместимый upgrade path: при отсутствии dedicated signing key ключ детерминированно выводится из существующего `API_KEY`, а отсутствие обоих ключей fail-closed с setup-подсказкой. - GitLab deploy webhook secret вынесен в отдельный nullable DB/API/UI field; OAuth `gitlab.secret` больше не принимается как webhook token, GitLab refresh-token webhooks без dedicated `webhookSecret` сохраняют legacy fallback, а dedicated webhook secret остается строгой границей. - Gitea OAuth authorization теперь запрашивает granular scopes `read:user read:repository read:organization` вместо GitHub-style `repo`. - Application и backup update paths сохраняют реальные секреты при сохранении redacted placeholders из edit forms. - Обновлен Monitoring UI: Docker Disk Usage получил inspect details, visible row limits, расширенный layout для раскрытого состояния и совместимость с upstream Tailwind v4/shadcn changes. - `getLocalServerIp` снова использует literal escaped dots в RFC1918 `grep -E` pattern. - PR Dokploy#4708 перебазирован на upstream/canary 91abc93; конфликты с upstream ssh-keyscan, ECR registry username casing и GitHub webhook owner matching разрешены без отката upstream behavior. - Next production build imports больше не требуют runtime Postgres password во время `phase-production-build`, но обычный production runtime без password/file продолжает fail-closed. - Signed-job export-map regression теперь принимает оба допустимых состояния package exports: source mode после `server:script` и dist mode после `server:build`, чтобы `pnpm test` проходил в CI-порядке `server:build -> test`. - `@dokploy/api` start script теперь указывает на фактический TypeScript output `dist/apps/api/src/index.js`, который получается при текущем `rootDir: "../.."`. Зачем: - Чтобы upstream PR можно было проверить как один coherent security/monitoring change без поломки существующих provider/deploy/update paths и без отката upstream изменений. - Чтобы service-scoped пользователи не теряли доступ к своим local container/log views и одновременно не получали Docker host access к чужим контейнерам. - Чтобы пользователи могли отменять deployment jobs после того, как целевой server стал inactive. - Чтобы существующие GitHub/GitLab deploy webhooks, schedule/deployment-management flows и edit/save формы с секретами не ломались после обновления. - Чтобы GitHub Actions `pnpm build` не требовал runtime DB secret при статическом Next build import, сохраняя runtime security boundary. - Чтобы GitHub Actions `pnpm test` не падал после production export switch, сохраняя проверку public signed-job subpath. - Чтобы API Docker image запускал реально собранный entrypoint, а не отсутствующий `dist/index.js`. Риски: - PR остается крупным и затрагивает много security-sensitive поверхностей; GitHub checks нужно дождаться на новом pushed head после очередного Codex follow-up. - Full local `pnpm test` после последнего follow-up не прошел только на `__test__/deploy/application.real.test.ts` real deploy/Nixpacks/Docker сценариях; focused regression suite и остальные PR gates прошли. - Первый локальный `pnpm build` упал на локальном Node 24.18.0 Next/Webpack cache `WasmHash`; после очистки ignored build cache повторный `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` прошел. - Ручной API-вызов с full container ID может быть отклонен, если Docker stats возвращает только short ID; текущий UI flow использует `stat.ID` из того же stats ответа и не затронут. Проверки: - Команды и результаты: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; CI test export-map focused Vitest passed 1 file / 11 tests in source-export mode and passed again after `pnpm server:build` in dist-export mode; latest Codex round focused Vitest passed 6 files / 58 tests; scoped Biome for latest 11 changed files passed; latest `git diff --check` and `git diff --cached --check` passed; `pnpm --filter @dokploy/api build` passed and `apps/api/dist/apps/api/src/index.js` exists; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed after clearing ignored build cache; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including the latest Arendt rerun pass for the 5 Codex threads. - Ограничения: full local `pnpm test` failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks/Docker scenarios; authenticated browser flows, production deploy, real VPS upgrade, GitHub-hosted checks on this amended head, and a separate Codex Security scan were not run in this commit. What: - Hardened server-side security boundaries for API/router/service paths, Docker and monitoring access, deploy/backup/restore flows, shell command builders, signed jobs, provider OAuth, redaction, and secret preservation. - Docker/monitoring endpoints now filter container stats and process lists through service-aware access checks; `server.getContainerProcesses` does not call `docker top` for foreign or unlinked container ids/names. - Local Docker container actions without `serverId` now use service-scoped container access without the owner/admin Docker host gate; remote `serverId` paths stay behind the owner/admin Docker host gate. - Local app-name Docker routes now preserve service-scoped access: service-read listing uses `service:read`, Docker app-name routes use `docker:read`, and remote `serverId` paths stay behind the owner/admin Docker host gate. - Cancel deployment API verification now preserves `requireActiveServer: false`, so stuck queued/running deployments can be cancelled even after the assigned server becomes inactive. - GitHub application/compose refresh-token deploy webhooks verify signatures against the raw request body while preserving the parsed body for the existing deploy logic. - Schedule and deployment queue signing now have a compatible upgrade path: when the dedicated signing key is missing, the key is deterministically derived from the existing `API_KEY`; missing both keys fails closed with setup guidance. - The GitLab deploy webhook secret is a separate nullable DB/API/UI field; OAuth `gitlab.secret` is no longer accepted as a webhook token, GitLab refresh-token webhooks without a dedicated `webhookSecret` keep the legacy fallback, and the dedicated webhook secret remains a strict boundary. - Gitea OAuth authorization now requests granular scopes `read:user read:repository read:organization` instead of the GitHub-style `repo` scope. - Application and backup update paths preserve real secrets when edit forms submit redacted placeholders. - Updated the Monitoring UI: Docker Disk Usage now has inspect details, visible row limits, expanded layout for the open state, and compatibility with upstream Tailwind v4/shadcn changes. - `getLocalServerIp` again uses literal escaped dots in the RFC1918 `grep -E` pattern. - Rebasing PR Dokploy#4708 onto upstream/canary 91abc93 resolved the upstream ssh-keyscan, ECR registry username casing, and GitHub webhook owner matching conflicts without reverting upstream behavior. - Next production build imports no longer require a runtime Postgres password during `phase-production-build`, while ordinary production runtime without a password/file still fails closed. - The signed-job export-map regression now accepts both valid package export states: source mode after `server:script` and dist mode after `server:build`, so `pnpm test` passes in the CI order `server:build -> test`. - The `@dokploy/api` start script now points to the emitted TypeScript output `dist/apps/api/src/index.js` produced by the current `rootDir: "../.."` config. Why: - So the upstream PR can be reviewed as one coherent security and monitoring change without breaking existing provider/deploy/update paths or reverting upstream changes. - So service-scoped users keep access to their own local container/log views without receiving Docker host access to foreign containers. - So users can cancel deployment jobs after the target server has become inactive. - So existing GitHub/GitLab deploy webhooks, schedule/deployment-management flows, and secret-bearing edit/save forms do not break after upgrade. - So GitHub Actions `pnpm build` does not require a runtime DB secret for static Next build imports while keeping the runtime security boundary. - So GitHub Actions `pnpm test` does not fail after the production export switch while still checking the public signed-job subpath. - So the API Docker image starts the entrypoint that was actually emitted instead of missing `dist/index.js`. Risks: - The PR remains large and touches many security-sensitive surfaces; GitHub checks need to rerun on the new pushed head after the next Codex follow-up. - Full local `pnpm test` after the latest follow-up failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks/Docker scenarios; the focused regression suite and other PR gates passed. - The first local `pnpm build` failed on a local Node 24.18.0 Next/Webpack cache `WasmHash` error; after clearing ignored build cache, the repeat `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed. - A manual API call with a full container ID can be rejected when Docker stats only returns the short ID; the current UI flow uses `stat.ID` from the same stats response and is not affected. Checks: - Commands and results: focused deploy/GitLab/auth/signed-job Vitest passed 5 files / 48 tests; focused registry Vitest passed 1 file / 13 tests; focused container process access Vitest passed 2 files / 23 tests; focused raw-body/schedule fallback Vitest passed 3 files / 30 tests; P2/P3 Docker route/IP focused Vitest passed 3 files / 25 tests; cancel inactive server focused Vitest passed 1 file / 11 tests; post-rebase GitHub webhook + signed-job focused Vitest passed 2 files / 15 tests; CI build-fix focused Vitest passed 3 files / 20 tests; CI test export-map focused Vitest passed 1 file / 11 tests in source-export mode and passed again after `pnpm server:build` in dist-export mode; latest Codex round focused Vitest passed 6 files / 58 tests; scoped Biome for latest 11 changed files passed; latest `git diff --check` and `git diff --cached --check` passed; `pnpm --filter @dokploy/api build` passed and `apps/api/dist/apps/api/src/index.js` exists; Next build-phase import smoke passed without `POSTGRES_PASSWORD`; production runtime no-secret import still failed closed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed; `POSTGRES_PASSWORD=dokploy pnpm build -> pnpm server:script` passed after clearing ignored build cache; focused real deploy suite passed in isolation with Docker Desktop in PATH; mandatory independent Agent Flow QA passed/pass-with-risks for follow-ups, including the latest Arendt rerun pass for the 5 Codex threads. - Limitations: full local `pnpm test` failed only in `__test__/deploy/application.real.test.ts` real deploy/Nixpacks/Docker scenarios; authenticated browser flows, production deployment, real VPS upgrade, GitHub-hosted checks on this amended head, and a separate Codex Security scan were not run in this commit.
|
Updated to I addressed the latest 5 Codex review threads, added regression coverage for each item, replied on the threads, and resolved them. Unresolved review thread count is now 0. GitHub checks on this head are reporting: @codex review |
|
CI update for The PR body has been updated with the final check state. Waiting for the requested |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 952bb380d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Что: - Зафиксирована высота expanded chart и добавлен отступ legend перед деталями Docker Disk Usage. - Добавлен regression test для expanded layout и tracked screenshot proof. Зачем: - Чтобы donut chart и legend не наезжали на карточки details в интерфейсе мониторинга. Риски: - Не выявлены для scoped UI layout helper; full suite и production deploy отдельно не запускались. Проверки: - Команды и результаты: `rtk env CI=true corepack pnpm --dir apps/dokploy exec vitest run --config __test__/vitest.config.ts __test__/monitoring/docker-disk-usage-layout.test.ts` passed, 1 file / 3 tests; `rtk env CI=true corepack pnpm exec biome check apps/dokploy/components/dashboard/monitoring/free/container/docker-disk-usage-layout.ts apps/dokploy/__test__/monitoring/docker-disk-usage-layout.test.ts` passed; `rtk git diff --check -- apps/dokploy/components/dashboard/monitoring/free/container/docker-disk-usage-layout.ts apps/dokploy/__test__/monitoring/docker-disk-usage-layout.test.ts docs/assets/docker-disk-usage-overlap/expanded-fixed.png` passed; browser proof and independent QA verifier passed in Agent Flow trace. - Ограничения: full build, full test suite, typecheck, repo-wide lint, production deploy, VPS/live Docker and wider viewport matrix were not run. What: - Fixed the expanded chart height and added legend spacing before Docker Disk Usage details. - Added a regression test for the expanded layout and a tracked screenshot proof. Why: - To prevent the donut chart and legend from overlapping details cards in the monitoring UI. Risks: - None identified for the scoped UI layout helper; full suite and production deploy were not run separately. Checks: - Commands and results: `rtk env CI=true corepack pnpm --dir apps/dokploy exec vitest run --config __test__/vitest.config.ts __test__/monitoring/docker-disk-usage-layout.test.ts` passed, 1 file / 3 tests; `rtk env CI=true corepack pnpm exec biome check apps/dokploy/components/dashboard/monitoring/free/container/docker-disk-usage-layout.ts apps/dokploy/__test__/monitoring/docker-disk-usage-layout.test.ts` passed; `rtk git diff --check -- apps/dokploy/components/dashboard/monitoring/free/container/docker-disk-usage-layout.ts apps/dokploy/__test__/monitoring/docker-disk-usage-layout.test.ts docs/assets/docker-disk-usage-overlap/expanded-fixed.png` passed; browser proof and independent QA verifier passed in Agent Flow trace. - Limitations: full build, full test suite, typecheck, repo-wide lint, production deploy, VPS/live Docker, and a wider viewport matrix were not run. (cherry picked from commit 5c2c143)
Что: - Возвращен `StrictHostKeyChecking=accept-new` в `GIT_SSH_COMMAND` для custom SSH clone при сохранении scoped `UserKnownHostsFile`. - Migration `0173_scope_sso_domains.sql` теперь backfill-ит существующие непустые SSO domains как `domainVerified = true` после добавления новой колонки. - Добавлены regression tests для SSH clone fallback и SSO domain backfill. Зачем: - Чтобы custom SSH deployments не ломались на first-time hosts, если `ssh-keyscan` не смог заранее записать ключ. - Чтобы существующие SSO providers не теряли auto-provisioning после upgrade из-за нового default `domainVerified = false`. Риски: - PR все еще ожидает новый GitHub-hosted CI и повторный Codex review после push. - Full local `pnpm test` и browser smoke для этого маленького follow-up не запускались. Проверки: - Команды и результаты: focused Vitest passed `2` files / `28` tests; scoped Biome passed with `Checked 3 files`; scoped `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed with no `packages/server/package.json` diff; independent Agent Flow QA returned `pass`. - Ограничения: full local `pnpm test`, full `pnpm build`, browser smoke, production deploy, real VPS upgrade и отдельный Codex Security scan не запускались. What: - Restored `StrictHostKeyChecking=accept-new` in `GIT_SSH_COMMAND` for custom SSH clone while keeping the scoped `UserKnownHostsFile`. - Migration `0173_scope_sso_domains.sql` now backfills existing non-empty SSO domains as `domainVerified = true` after adding the new column. - Added regression tests for the SSH clone fallback and SSO domain backfill. Why: - So custom SSH deployments do not break on first-time hosts when `ssh-keyscan` cannot pre-populate a host key. - So existing SSO providers do not lose auto-provisioning after upgrade because the new default is `domainVerified = false`. Risks: - The PR still needs fresh GitHub-hosted CI and another Codex review after push. - Full local `pnpm test` and browser smoke were not run for this small follow-up. Checks: - Commands and results: focused Vitest passed `2` files / `28` tests; scoped Biome passed with `Checked 3 files`; scoped `git diff --check` passed; `pnpm format-and-lint` passed with 3 existing warnings; `pnpm typecheck` passed; `pnpm server:build -> pnpm server:script` passed with no `packages/server/package.json` diff; independent Agent Flow QA returned `pass`. - Limitations: full local `pnpm test`, full `pnpm build`, browser smoke, production deployment, real VPS upgrade, and a separate Codex Security scan were not run.
|
Updated to I addressed the latest 2 Codex P2 threads from the
I replied to and resolved both review threads. Unresolved review thread count is now 0. Local verification passed: focused Vitest 2 files / 28 tests, scoped Biome, scoped diff check, GitHub checks are rerunning for this head now. @codex review |
|
CI update for The PR body has been updated with the final check state. Waiting for the requested |
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a0612e8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!provider) { | ||
| return true; |
There was a problem hiding this comment.
Reject missing provider headers when webhook secrets exist
When a GitHub/GitLab deploy webhook is sent without x-github-event/x-gitlab-event, getProviderByHeader returns null and this branch authenticates the request. For GitHub/GitLab applications or compose services that store a webhook secret, anyone who obtains the refresh-token URL can omit the provider header and submit a forged body with matching branch/watch paths, bypassing the new signature/token validation before the shared helper lets both deploy endpoints queue a deployment. Treat missing provider headers as unauthenticated whenever the resolved source provider has a webhook secret.
Useful? React with 👍 / 👎.
Что: - Добавлен application ENV upsert API, UI reveal/preserve flow для env secrets и безопасная обработка тела custom route с лимитом 1 MiB. - Добавлено сохранение serverId в application/compose deploy queue payloads и общий helper для ENV upsert redeploy jobs. - Добавлены регрессионные тесты для ENV upsert, redaction/reveal, remote queue partitioning и oversized ENV upsert request bodies. Зачем: - Чтобы обновления ENV не затирали секреты redacted placeholders, а remote deployments и redeploy jobs попадали в правильную server queue partition. Риски: - Full repository typecheck остается заблокирован существующим baseline `TS5101` в `packages/server/tsconfig.json`. Проверки: - Команды и результаты: `rtk proxy corepack pnpm exec biome check --no-errors-on-unmatched --files-ignore-unknown=true <29 changed files>` - passed; `rtk proxy corepack pnpm --dir apps/dokploy exec vitest run --config __test__/vitest.config.ts __test__/env/upsert.test.ts __test__/env/application-env-upsert.test.ts __test__/security/env-redaction.test.ts __test__/security/service-environment.test.ts __test__/server/application-env-upsert-body.test.ts __test__/deploy/github-webhook-handler.test.ts` - 6 files / 24 tests passed; `rtk git diff --check` - passed; `rtk git diff --cached --check` - passed; filtered `tsc --noEmit --ignoreDeprecations 6.0` for changed-file patterns in `dokploy` and `@dokploy/server` - no diagnostics; independent reviewer subagent verdict - pass. - Ограничения: `rtk proxy corepack pnpm run typecheck` fails before changed-file diagnostics on existing `packages/server/tsconfig.json(24,3): TS5101 Option 'baseUrl' is deprecated`; full build and full test suite were not run. What: - Added the application ENV upsert API, UI reveal/preserve flow for env secrets, and safe custom route body handling with a 1 MiB limit. - Preserved serverId in application/compose deploy queue payloads and added a shared helper for ENV upsert redeploy jobs. - Added regression tests for ENV upsert, redaction/reveal, remote queue partitioning, and oversized ENV upsert request bodies. Why: - So ENV updates do not overwrite secret redacted placeholders, and remote deployments and redeploy jobs land in the correct server queue partition. Risks: - Full repository typecheck remains blocked by the existing baseline `TS5101` in `packages/server/tsconfig.json`. Checks: - Commands and results: `rtk proxy corepack pnpm exec biome check --no-errors-on-unmatched --files-ignore-unknown=true <29 changed files>` - passed; `rtk proxy corepack pnpm --dir apps/dokploy exec vitest run --config __test__/vitest.config.ts __test__/env/upsert.test.ts __test__/env/application-env-upsert.test.ts __test__/security/env-redaction.test.ts __test__/security/service-environment.test.ts __test__/server/application-env-upsert-body.test.ts __test__/deploy/github-webhook-handler.test.ts` - 6 files / 24 tests passed; `rtk git diff --check` - passed; `rtk git diff --cached --check` - passed; filtered `tsc --noEmit --ignoreDeprecations 6.0` for changed-file patterns in `dokploy` and `@dokploy/server` - no diagnostics; independent reviewer subagent verdict - pass. - Limitations: `rtk proxy corepack pnpm run typecheck` fails before changed-file diagnostics on the existing `packages/server/tsconfig.json(24,3): TS5101 Option 'baseUrl' is deprecated`; full build and full test suite were not run.
Что: - Смержил текущий upstream/canary в agentHits-PR и разрешил конфликты без удаления локальных access/security guards. - Объединил canary cloud-limit изменения с существующими ENV upsert, redaction, backup, deployment и placement checks. - Добавил точечные исправления, найденные typecheck/Vitest после merge resolution. Зачем: - Обновить ветку PR до актуального canary и сохранить готовые изменения ветки для дальнейшей проверки и публикации. Риски: - Полный локальный test suite не запускался успешно из-за environment-sensitive real deploy tests; targeted regression suite прошел. - `pnpm run format-and-lint` в этой среде обработал 0 файлов через `biome check .`; repo-wide Biome был проверен через явные top-level paths. Проверки: - Команды и результаты: `rtk proxy corepack pnpm install --frozen-lockfile` - passed; `rtk proxy corepack pnpm run typecheck` - passed; `rtk proxy corepack pnpm exec vitest run --config __test__/vitest.config.ts __test__/environment/environment-secret-boundary.test.ts __test__/backup/rclone-command-boundary.test.ts __test__/backups/redact-credentials.test.ts __test__/utils/hostname-validation.test.ts __test__/server/server-setup.test.ts __test__/security/env-redaction.test.ts __test__/security/service-environment.test.ts __test__/server/application-env-upsert-body.test.ts` - passed, 8 files / 54 tests; `rtk proxy corepack pnpm exec biome check apps/dokploy/server/api/routers/deployment.ts apps/dokploy/server/api/routers/environment.ts apps/dokploy/server/api/routers/user.ts apps/dokploy/__test__/environment/environment-secret-boundary.test.ts packages/server/src/utils/backups/utils.ts` - passed; `rtk proxy corepack pnpm exec biome check apps packages .github package.json biome.json --no-errors-on-unmatched --files-ignore-unknown=true` - passed with 3 existing warnings; `rtk git diff --check` and `rtk git diff --cached --check` - passed; Agent Flow reviewer - pass-with-risks, no blockers. - Ограничения: `pnpm run format-and-lint` failed because Biome processed 0 files for `biome check .` in this environment; an accidental broad Vitest run hit existing environment-sensitive failures and was not used as the final signal. What: - Merged the current upstream/canary into agentHits-PR and resolved conflicts without dropping local access/security guards. - Combined canary cloud-limit changes with the existing ENV upsert, redaction, backup, deployment, and placement checks. - Added focused fixes found by typecheck/Vitest after merge resolution. Why: - Bring the PR branch up to the current canary while preserving the branch's ready changes for further validation and publication. Risks: - The full local test suite was not successfully run because of environment-sensitive real deploy tests; the targeted regression suite passed. - `pnpm run format-and-lint` processed 0 files through `biome check .` in this environment; repo-wide Biome was checked through explicit top-level paths. Checks: - Commands and results: `rtk proxy corepack pnpm install --frozen-lockfile` - passed; `rtk proxy corepack pnpm run typecheck` - passed; `rtk proxy corepack pnpm exec vitest run --config __test__/vitest.config.ts __test__/environment/environment-secret-boundary.test.ts __test__/backup/rclone-command-boundary.test.ts __test__/backups/redact-credentials.test.ts __test__/utils/hostname-validation.test.ts __test__/server/server-setup.test.ts __test__/security/env-redaction.test.ts __test__/security/service-environment.test.ts __test__/server/application-env-upsert-body.test.ts` - passed, 8 files / 54 tests; `rtk proxy corepack pnpm exec biome check apps/dokploy/server/api/routers/deployment.ts apps/dokploy/server/api/routers/environment.ts apps/dokploy/server/api/routers/user.ts apps/dokploy/__test__/environment/environment-secret-boundary.test.ts packages/server/src/utils/backups/utils.ts` - passed; `rtk proxy corepack pnpm exec biome check apps packages .github package.json biome.json --no-errors-on-unmatched --files-ignore-unknown=true` - passed with 3 existing warnings; `rtk git diff --check` and `rtk git diff --cached --check` - passed; Agent Flow reviewer - pass-with-risks, no blockers. - Limitations: `pnpm run format-and-lint` failed because Biome processed 0 files for `biome check .` in this environment; an accidental broad Vitest run hit existing environment-sensitive failures and was not used as the final signal.
Что: - Ограничил чтение текущего compose в `compose.update` только случаями, когда вход содержит redacted placeholder для `env`, `composeFile` или `customGitUrl`. - Добавил регрессионный тест, что обычный non-source compose update не включает current provider guard, а redacted placeholders продолжают сохранять реальные значения. Зачем: - Починить падение `pr-check (test)` в PR Dokploy#4708 без ослабления защиты deploy source credentials и без перезаписи скрытых compose-секретов. Риски: - Не выявлены; отдельный кейс `customGitUrl` не добавлен как отдельный тест, но покрыт тем же списком placeholder-полей и helper-логикой. Проверки: - Команды и результаты: `rtk proxy corepack pnpm exec biome check --write apps/dokploy/server/api/routers/compose.ts apps/dokploy/__test__/git-provider/deploy-source-credential-access.test.ts` - passed; `rtk proxy corepack pnpm exec vitest run --config __test__/vitest.config.ts __test__/git-provider/deploy-source-credential-access.test.ts` - passed, 1 file / 32 tests; `rtk proxy corepack pnpm run typecheck` - passed; `rtk git diff --check` and `rtk git diff --cached --check` - passed; Agent Flow reviewer - pass. - Ограничения: полный `pnpm test` локально не запускался повторно из-за real deploy окружения; CI ранее показал, что в этом job падал только исправленный test file. What: - Limited the current compose read in `compose.update` to inputs that contain a redacted placeholder for `env`, `composeFile`, or `customGitUrl`. - Added a regression test that a normal non-source compose update does not trigger the current provider guard, while redacted placeholders still preserve real values. Why: - Fix the `pr-check (test)` failure in PR Dokploy#4708 without weakening deploy source credential protection or overwriting hidden compose secrets. Risks: - None identified; a separate `customGitUrl` case was not added as its own test, but it is covered by the same placeholder field list and helper logic. Checks: - Commands and results: `rtk proxy corepack pnpm exec biome check --write apps/dokploy/server/api/routers/compose.ts apps/dokploy/__test__/git-provider/deploy-source-credential-access.test.ts` - passed; `rtk proxy corepack pnpm exec vitest run --config __test__/vitest.config.ts __test__/git-provider/deploy-source-credential-access.test.ts` - passed, 1 file / 32 tests; `rtk proxy corepack pnpm run typecheck` - passed; `rtk git diff --check` and `rtk git diff --cached --check` - passed; Agent Flow reviewer - pass. - Limitations: full `pnpm test` was not rerun locally because of the real deploy environment; CI previously showed that only the fixed test file failed in this job.
What is this PR about?
This PR collects security hardening, monitoring UX, safe env update, signed worker-job boundaries, and regression coverage work into one upstream review branch for
Dokploy/dokploy:canary.Fresh readiness pass:
upstream/canary91abc93c102d0015bfce78ca87e6f6a785a086f606a0612e8cc6417819cfc1f357c99d16a4fb5894agentHits-pr.agent-work/runs/2026-07-02-pr4708-codex-p2-ssh-ssoLatest Update - 06a0612
Addressed the latest Codex review on
952bb380d:StrictHostKeyChecking=accept-newin the custom SSH cloneGIT_SSH_COMMANDwhile keeping the scopedUserKnownHostsFile.domainVerified = truein0173_scope_sso_domains.sql, while new providers still default to unverified.Verification for
06a0612e8:2files /28tests.Checked 3 files; SQL file is not handled by Biome).git diff --checkpassed.pnpm format-and-lintpassed with the existing 3 warnings only.pnpm typecheckpassed.pnpm server:build -> pnpm server:scriptpassed and left nopackages/server/package.jsondiff.pass.format,pr-check (build),pr-check (test), andpr-check (typecheck).Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
Not linked.
Screenshots (if applicable)
No generated screenshot files are added to this upstream diff beyond the tracked Docker Disk Usage proof asset already included in the branch.
Detailed Changes
Security and access boundaries
API_KEYcompatibility fallback, and fail-closed missing-key guidance.Docker, monitoring, and deployment behavior
serverIdis omitted, while remoteserverIdpaths remain behind the Docker host gate.requireActiveServer: false, so stuck queued/running deployments can be cancelled even if the assigned server is inactive.Build/package and upgrade boundaries
apps/apibuild output aligned with its Docker runtime by startingnode dist/apps/api/src/index.js, matching the currentrootDir: "../.."TypeScript output.StrictHostKeyChecking=accept-new.domainVerifiedmigration while new domains default to unverified.Verification Summary
Latest local checks for
06a0612e8are listed above. Earlier PR rounds also passed focused deploy/GitLab/auth/signed-job tests, registry security tests, container process access tests, raw-body/schedule fallback tests, Docker route/IP tests, cancel inactive server tests, API build entrypoint tests,format-and-lint,typecheck,server:build -> server:script, and mandatory independent Agent Flow QA for each file-changing follow-up.Known local limitation from earlier rounds:
CI=true corepack pnpm testfailed only in__test__/deploy/application.real.test.tsreal deploy/Nixpacks/Docker scenarios: 1 failed file, 4 failed tests, 156 passed files, 1370 passed tests, 1 skipped. Focused review regressions and GitHub-hosted checks are the current handoff gates.Risks
REVIEW_REQUIREDuntil Codex/maintainer re-review completes.