Repository navigation
fix: scope container files, service logs and traefik files access - #26
Merged
Merged
Conversation
Что: - docker.listContainerFiles, readContainerFile, writeContainerFile и deleteContainerFile теперь проходят assertDockerServerAccess (только owner/admin и доступный сервер) и resolveAuthorizedContainerId (локальный контейнер должен принадлежать сервису, к которому есть доступ). - writeContainerFile требует docker.write, deleteContainerFile требует docker.delete вместо docker.read. - docker.getEvents использует ту же проверку owner/admin и доступа к серверу. - dockerVolume.listVolumeFiles, readVolumeFile, writeVolumeFile, deleteVolumeFile и removeVolume используют assertDockerServerAccess; запись требует docker.write, удаление docker.delete. Зачем: - Раньше эти процедуры проверяли только docker.read и принадлежность сервера организации, поэтому пользователь с docker.read мог читать, менять и удалять файлы в любом контейнере или volume и удалять volumes. Риски: - Owner/admin больше не могут открыть файлы локальных контейнеров, которые не привязаны к сервису Dokploy (так же, как уже работают restart/upload). - Пользователи с кастомными ролями, у которых есть docker.read, больше не видят файлы контейнеров и volumes и Docker events. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/docker/docker-router-security.test.ts __test__/docker/docker-volume-router-security.test.ts - все тесты прошли; новые тесты падают на старом коде (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок. - Ограничения: UI проверен только чтением кода (файловый менеджер есть только на странице Docker, где список контейнеров уже требует owner/admin); на реальном Docker не запускалось. What: - docker.listContainerFiles, readContainerFile, writeContainerFile and deleteContainerFile now go through assertDockerServerAccess (owner/admin plus an accessible server) and resolveAuthorizedContainerId (a local container must belong to a service the caller may access). - writeContainerFile requires docker.write and deleteContainerFile requires docker.delete instead of docker.read. - docker.getEvents uses the same owner/admin and server access check. - dockerVolume.listVolumeFiles, readVolumeFile, writeVolumeFile, deleteVolumeFile and removeVolume use assertDockerServerAccess; writes require docker.write, deletes docker.delete. Why: - These procedures only checked docker.read and that the server belongs to the organization, so a docker.read holder could read, change and delete files in any container or volume and remove volumes. Risks: - Owners/admins can no longer open files of local containers that are not bound to a Dokploy service (the same rule restart/upload already follow). - Custom roles with docker.read no longer see container/volume files or Docker events. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/docker/docker-router-security.test.ts __test__/docker/docker-volume-router-security.test.ts - all passed; the new tests fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors. - Limitations: UI checked by reading code only (the file explorer lives on the Docker page, whose container list already requires owner/admin); not run against a real Docker host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Что:
- readLogs в роутерах application, compose, postgres, mysql, mariadb, mongo, redis и libsql теперь вызывает checkServicePermissionAndAccess с { service: ["read"], logs: ["read"] } вместо checkServiceAccess(..., "read").
Зачем:
- Право logs.read есть в модели прав, но эти процедуры его не проверяли: кастомная роль без logs.read все равно могла читать логи контейнеров сервиса.
Риски:
- Кастомные роли без logs.read теряют доступ к логам сервисов. Статические роли owner, admin и member имеют logs.read, их поведение не меняется.
Проверки:
- Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/logs/read-logs-permission.test.ts - 8 тестов прошли; на старом коде все 8 падают (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок.
- Ограничения: логи через WebSocket (server/wss) не менялись.
What:
- readLogs in the application, compose, postgres, mysql, mariadb, mongo, redis and libsql routers now calls checkServicePermissionAndAccess with { service: ["read"], logs: ["read"] } instead of checkServiceAccess(..., "read").
Why:
- logs.read exists in the permission model but these procedures never checked it, so a custom role without logs.read could still read a service's container logs.
Risks:
- Custom roles without logs.read lose access to service logs. The static owner, admin and member roles all grant logs.read, so their behavior is unchanged.
Checks:
- Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/logs/read-logs-permission.test.ts - 8 tests passed; all 8 fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors.
- Limitations: WebSocket log streaming (server/wss) is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Что: - resolveTraefikConfigPath отклоняет acme.json, файлы *.key и все внутри dynamic/certificates, поэтому settings.readTraefikFile и settings.updateTraefikFile не могут читать или перезаписывать их ни локально, ни на удаленном сервере. - settings.readDirectories скрывает эти файлы и папку certificates из дерева. - readDirectories, readTraefikFile и updateTraefikFile без serverId требуют доступ к локальному хосту (owner/admin, assertLocalHostAccess), как другие процедуры локального хоста. Зачем: - Пользователь с traefikFiles.read мог прочитать acme.json и приватные ключи сертификатов, а без serverId не было проверки роли для локального хоста. Риски: - Эти файлы больше нельзя открыть или изменить в редакторе Traefik даже owner/admin; сертификаты по-прежнему управляются через раздел сертификатов. - Кастомные роли с traefikFiles.* теряют доступ к файлам Traefik локального хоста; удаленные серверы работают как раньше. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/traefik/traefik-file-boundary.test.ts __test__/settings/docker-server-boundary.test.ts - все тесты прошли; новые тесты падают на старом коде (исходники временно возвращены к origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check по измененным файлам - без ошибок. - Ограничения: симлинки внутри папки Traefik не разрешаются (проверка идет по пути); на реальном сервере не проверялось. What: - resolveTraefikConfigPath rejects acme.json, *.key files and anything under dynamic/certificates, so settings.readTraefikFile and settings.updateTraefikFile can no longer read or overwrite them, locally or on a remote server. - settings.readDirectories hides these files and the certificates directory from the tree. - readDirectories, readTraefikFile and updateTraefikFile without a serverId require local host access (owner/admin, assertLocalHostAccess), like the other local host procedures. Why: - A traefikFiles.read holder could read acme.json and certificate private keys, and without a serverId there was no role check for the local host. Risks: - These files can no longer be opened or edited in the Traefik editor, even by owners/admins; certificates are still managed through the certificates section. - Custom roles with traefikFiles.* lose access to local host Traefik files; remote servers work as before. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/traefik/traefik-file-boundary.test.ts __test__/settings/docker-server-boundary.test.ts - all passed; the new tests fail on the old code (sources temporarily reset to origin/AgentHits-Dev); bun run typecheck - exit 0; bunx biome check on changed files - no errors. - Limitations: symlinks inside the Traefik directory are not resolved (the check is path based); not checked on a real server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Included in #31 (super password), which merged these commits and replaced the hard denies with super-session gates. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR about?
Access scoping for container files, volume files, service logs and Traefik files. Three commits:
docker.ts,docker-volume.ts)listContainerFiles,readContainerFile,writeContainerFile,deleteContainerFilenow use the same checks as the other Docker procedures:assertDockerServerAccess(owner/admin and an accessible server) andresolveAuthorizedContainerId(a local container must belong to a service the caller may access). Write needsdocker.write, delete needsdocker.delete(both weredocker.read).getEventsgets the same owner/admin and server check.listVolumeFiles,readVolumeFile,writeVolumeFile,deleteVolumeFile,removeVolumeuseassertDockerServerAccess; write needsdocker.write, delete/remove needdocker.delete.application,compose,postgres,mysql,mariadb,mongo,redis,libsql)readLogsnow callscheckServicePermissionAndAccess(ctx, id, { service: ["read"], logs: ["read"] })instead ofcheckServiceAccess(ctx, id, "read"). The static owner, admin and member roles all grantlogs.read.settings.ts,utils/traefik/application.ts)resolveTraefikConfigPathrejectsacme.json,*.keyand anything underdynamic/certificates, for both read and write, local and remote.readDirectorieshides these entries from the tree.readDirectories,readTraefikFileandupdateTraefikFilewithout aserverIdrequire local host access (assertLocalHostAccess, owner/admin).Who is affected:
acme.json,*.keyand certificate files are no longer visible in the Traefik editor.docker.*ortraefikFiles.*, and they havelogs.read).docker.readalone no longer opens container/volume files or Docker events; logs needlogs.read; local host Traefik files need owner/admin.Checks
CI=true bunx vitest run --config __test__/vitest.config.ts __test__/docker/docker-router-security.test.ts __test__/docker/docker-volume-router-security.test.ts __test__/settings/docker-server-boundary.test.ts __test__/traefik/traefik-file-boundary.test.ts __test__/logs/read-logs-permission.test.ts: 71 passed.origin/AgentHits-Dev, the same run gives 33 failed / 38 passed: every new test fails on the old code.docker,settings,traefik,logs,security,permissions,wss,swarm,server): 66 files, 508 tests passed.deploy/application.real.test.ts, 5 tests; it needs a local Docker socket and fails locally as expected).bun run typecheck: exit 0.bunx biome checkon the changed files: no errors.Rollout notes
acme.jsonandcertificates/should be gone from it).server/wssstill checks only service read access for service-scoped containers.🤖 Generated with Claude Code