fix(server): read task storage from the tasks manager, not the request

[vmrm]

Summary

The release and publish tasks read req.Storage after the request handler has returned. When trdl runs as a builtin backend (Stronghold compiles it in), the Vault router resets req.Storage to nil in the deferred block of routeCommon as soon as the handler returns, so the first storage read inside the task dereferences nil and the panic takes the whole Vault process down. The task already receives its own storage from the tasks manager; use it for every storage read inside the task.

Key changes

• server/path_release.go: the task reads trusted PGP keys, ELF signing settings and the build options' Storage from the storage parameter of the task, not from the captured req.Storage (three sites).
• server/path_publish.go: the same for the trusted PGP keys read (one site; the storage.Put at the end of the task already used the parameter).
• server/task_storage_test.go: a suite that captures the task from RunTask, lets HandleRequest return, sets req.Storage = nil the way the router does, and runs the task on the storage the tasks manager provides. The release task must reach the build stage (it fails on a deliberately absent buildkitd socket), the publish task must reach the channels config parse; either one panicking fails the test.

Why

Observed on a Stronghold 1.19 stand, where trdl is a builtin secrets engine: trdl/release answers with a task UUID, the task clones the repository, and then pgp.GetTrustedPGPPublicKeys(ctx, {0x0, 0x0}) panics at path_release.go:133. WrapTaskFunc re-panics anything but "send on closed channel", so the process exits with code 2 and the active replica restarts; the task stays RUNNING in storage forever. publish fails the same way at its PGP read.

The defect is latent upstream because both ways the code is exercised keep req.Storage alive:

• as an external plugin, req.Storage is a GRPCStorageClient built per request in the plugin process (sdk/plugin/grpc_backend_server.go), which the core's reset never reaches — this is what task server:setup-vault-local and the e2e flow use;
• the unit tests mock RunTask and never execute the task.

In builtin mode the request object is the core's own, and vault/router.go (upstream v1.19.0, lines 646/763) attaches re.storageView before routing and sets req.Storage = nil in the deferred reset. The storage the tasks manager hands to the task is m.Storage, the mount's storage view kept from the first request, which is what the worker's own status writes already use in both modes.

Review focus / risks

• Only the four reads inside the two task closures change; the handler-time reads (getConfiguration, git credentials, GetRepository, the RunTask argument itself) still use req.Storage, which is valid while the handler runs.
• The test clones from a local path, so it needs git on PATH (go-git's file transport runs git-upload-pack); the existing e2e flow already requires git.
• elf_signing.GetSettings reads storage only on linux && amd64 && cgo; on other platforms it returns before touching storage, so the ELF site is exercised by the test only on the Linux CI build (verified on the CI image, see the methodology comment).

🤖 Generated with Claude Code

[vmrm]

Verification

• task --yes server:lint — green (golangci-lint v2.11.0 + prettier), macOS arm64.
• task --yes server:test:unit — green, 256 tests, macOS arm64.
• go test -race -count=1 -run TestTaskStorage ./ inside golang:1.25.12-bookworm on linux/amd64 with the server:deps:install:c package set (the task target was not installed in the throwaway container, so the ginkgo wrapper was skipped and the package test was run directly) — green on the fixed tree.

Mutation evidence

Each row reverts one of the four changed reads back to req.Storage on top of the fixed tree; the tree was restored from git and checked clean (git diff --quiet HEAD) after every round.

Mutation	Expected evidence	Result
release: GetTrustedPGPPublicKeys(ctx, req.Storage)	TestReleaseTaskDoesNotUseRequestStorage fails on require.NotPanics	Killed (macOS)
release: elf_signing.GetSettings(ctx, req.Storage)	same test fails on require.NotPanics	Survived on macOS, Killed on linux/amd64 — GetSettings returns before touching storage unless linux && amd64 && cgo, so only the Linux build reaches the read; CI's unit job is that build
release: Storage: req.Storage in BuildReleaseArtifactsOpts	nil dereference in the build goroutine (secrets.GetSecrets)	Killed — unrecoverable panic in the goroutine, the test binary exits with panic: runtime error: invalid memory address
publish: GetTrustedPGPPublicKeys(ctx, req.Storage)	TestPublishTaskDoesNotUseRequestStorage fails on require.NotPanics	Killed (macOS)

Rounds 1 and 4 were applied together (they hit different scenarios; each test failed on its own site and neither masked the other). The third row is a red test binary rather than a red assertion because the panic happens in a goroutine the task starts; it still fails the package.

Not run

• The real builtin path (Vault core routing a request into a compiled-in trdl backend) is not reproduced in this repository's tests; the suite simulates the router's req.Storage = nil reset. The observed crash on a Stronghold 1.19 stand, with the trace ending in pgp.GetTrustedPGPPublicKeys({...}, {0x0, 0x0}) at path_release.go:133, is the field evidence; the green run of the same stand on a build carrying this commit is pending and will be reported here.
• The external-plugin path is unchanged by the diff (the task's storage parameter is the same object the worker's status writes already use), so task e2e:test:e2e:flow-vault was not re-run for this PR; the CI workflow runs it.

[vmrm]

Field evidence from the Stronghold stand

Stronghold 1.19 (release-1.19 + this commit, trdl compiled in as a builtin secrets engine, three replicas), the same engine configuration that crashed the active replica before:

Build	trdl/git-signatures/release git_tag=v0.0.1	Replica restarts
without this fix	task stays RUNNING forever; active replica exits with code 2 on pgp.GetTrustedPGPPublicKeys({...}, {0x0, 0x0}) at path_release.go:133	1 per attempt, leadership moves
with this fix (de81dcd on top of the pinned fork)	task 5c2fcccc…: FAILED in one second with not enough verified PGP signatures (the tag object was unsigned at the time); task 6b89b4a5… after signing the tag object: passes signature verification, reads trdl.yaml, starts the build, the kubernetes builder pod trdl-builder-1efa9b78… is created and removed	0 across both attempts

The second task then fails inside the build on mkdir /.docker: read-only file system — a separate builtin-only defect in the BuildKit auth provider's token-seed directory, fixed in #427.

[vmrm]

Green run on the stand

Same stand, Stronghold release-1.19 with this commit and #427 compiled in (vmrm/trdl@b50a464), three replicas.

• trdl/git-signatures/release git_tag=v0.0.1 → task 04054dd3… SUCCEEDED in 3m00s (created 21:42:01Z, modified 21:45:02Z), replica restarts 0.
• Builder pod trdl-builder-4983f4e0… in trdl-build: scheduled 21:42:01Z, buildkitd started 21:42:03Z, stopped 21:44:56Z, namespace empty afterwards.
• TUF repository: targets.json version 1 now lists releases/0.0.1/{linux,darwin}-{amd64,arm64}/bin/git-signatures and the four signatures/0.0.1/…/git-signatures.sig; the linux-amd64 target downloads at the recorded length (4378786) with a matching sha512 and is a statically linked Go ELF.

Every storage read the task makes now goes through the storage the tasks manager provides; the only remaining oddity is that the stored task log ends mid-line (Docker config dir "/.docker" is not wri), which looks like a separate capture limit in the task log and is not touched here.

[vmrm]

Independent review (Codex, static pass) and what changed

Accepted and fixed in 296f7cd:

• The release scenario proved only that req.Storage is not dereferenced; a task reading a fresh empty storage would have passed too. The storage handed to the task is now a counting wrapper and the test requires at least one List/Get through it. Mutation: the task swaps the provided storage for &logical.InmemStorage{} right at the top of the closure → TestReleaseTaskDoesNotUseRequestStorage fails with "the task must read through the storage it was given". Killed.
• The release scenario reached config.LoadDefaultConfigFile and could create the developer's ~/.docker as a side effect; the docker config dir is pinned to a temp dir for the test and restored.

Noted, not addressed here: the tests clone through go-git's file transport, which needs git on PATH (the e2e flow already requires it, and CI has it); the publish scenario stops at the channels config parse, so the final storage.Put — which was already using the task's storage before this PR — is not exercised by it; the tasks manager worker runs on context.Background() without a Clean hook, so queued tasks can outlive a plugin reload/unmount (pre-existing, unrelated to which storage the task reads).

[vmrm]

Code Review Report (independent pass, repository review skill, subagent)

Base: origin/main @ a10e3b9 (fix(server): harden the kubernetes builder pod (#425)); origin/main is an ancestor of the PR head 296f7cd.
Diff: [3 files, +148/-4 lines]

Автор диффа — агент, поэтому к обычной процедуре применён agent-code-review. Проход независим от сессии автора: рассуждения из PR брались как утверждения для проверки.

Verdict

• Technical: Исправление корректно и минимально — четыре чтения внутри двух замыканий задач переведены на параметр storage (server/path_release.go:133,155,172, server/path_publish.go:153), чтения на этапе обработчика (path_release.go:83,92,113,118, path_publish.go:69,78,92,105,110) остались на req.Storage, что верно: по vault/router.go@v1.19.0 req.Storage живёт от строки 646 до deferred-сброса на строке 763, который срабатывает после re.backend.HandleRequest (791). Класс дефекта выметен целиком: в server/ три вызова RunTask, третье замыкание (server/periodic.go:74-82) уже читает через storage; захваченный publisherRepository хранилище не удерживает (server/pkg/publisher/repository.go:43-49).
• Product: Пользовательской поверхности изменение не касается; во внешнем режиме поведение не меняется (sdk/plugin/grpc_backend_server.go:140 — свежий GRPCStorageClient на каждый запрос), в builtin-режиме устраняет падение процесса Vault на release/publish.
• Risk: Низкий. Тесты доказывают дискриминацию по пяти из семи мутаций; одна выживает на macOS по построению (ELF), одна — на строке, которой PR не касается.

DoD Criteria

Criteria	Inferred?	Met?	Evidence
Четыре чтения в замыканиях задач идут через параметр storage	yes	✅	server/path_release.go:133,155,172, server/path_publish.go:153
Чтения на этапе обработчика не тронуты	yes	✅	server/path_release.go:83,92,113,118; server/path_publish.go:69,78,92,105,110; router.go@v1.19.0:646,763,791
Ни одно другое замыкание задачи в server/ не читает req.Storage	yes	✅	три вызова RunTask; server/periodic.go:74-82,102-117 уже на storage
Тест воспроизводит сброс роутера и запускает задачу на хранилище менеджера задач	yes	✅	server/task_storage_test.go:80-99; мутации M1/M4 убиты
Тест требует хотя бы одно чтение через выданное хранилище	yes	✅	server/task_storage_test.go:98; мутации M5/M6 убиты
Release-задача доходит до стадии сборки	yes	✅	task_storage_test.go:63 ↔ server/pkg/docker/build.go:131
Publish-задача доходит до разбора trdl_channels.yaml	yes	✅	task_storage_test.go:75 ↔ server/path_publish.go:170
ELF-чтение (path_release.go:155) проверяется тестом на Linux CI	yes	⚠️	на macOS GetSettings выходит до чтения (server/pkg/elf_signing/storage.go:39-41); Linux-прогон в ревью недоступен
Тест не создаёт ~/.docker у разработчика	yes	✅	server/task_storage_test.go:48-50
Ворота зелёные	yes	⚠️	golangci-lint 0 issues.; prettier не выполнился (удалённый docker-хост недоступен); server:test:unit — Test Suite Passed

Issues

• Critical — нет.
• Major — нет.
• Minor — server/task_storage_test.go:21-24,101-104: два новых дублёра без var _ Iface = (*T)(nil) (CODESTYLE.md). Существующие дублёры репозитория такого чека тоже не имеют.
• Minor — server/task_storage_test.go:113: хелпер initGitRepository в файле сьюта, AGENTS.md требует helpers_test.go; в корневом пакете такого файла нет, моки лежат в backend_test.go.
• Minor — server/task_storage_test.go:116,125: клон через file-транспорт go-git требует git в PATH; без него сценарий падает с «unable to clone git repository», а не с сообщением о недостающем инструменте.
• Minor — server/path_publish.go:197: финальный storage.Put publish-задачи тестом не достигается (мутация M7 выжила). Строка этим PR не менялась.

Risks

№	Risk	Type	Likelihood	Severity	Location	Circumstances	Consequences	Recommendation
1	Паника внутри горутины сборки не перехватывается WrapTaskFunc	Operational	Unlikely	High	server/path_release.go:165-185; server/pkg/tasks_manager/actions.go:101-112	recover покрывает только горутину задачи; docker.BuildReleaseArtifacts — во вложенной горутине без recover (мутация M3: процесс умирает целиком)	Любая будущая паника на этом пути в builtin-режиме снова уронит процесс Vault	Вне скоупа PR; follow-up: recover в горутине на :165 с переводом в errCh
2	ELF-чтение верифицируется только Linux-джобой CI	Technical	Possible	Low	server/path_release.go:155; server/pkg/elf_signing/storage.go:38-41	локальный прогон на macOS не отличает storage от req.Storage на этой строке	регрессия дойдёт до CI, но не будет поймана локально	ничего не менять; unit-джоба CI — единственные ворота для этой строки
3	Release-сценарий опирается на быстрый отказ на отсутствующем unix-сокете	Technical	Possible	Low	server/task_storage_test.go:59,63; server/pkg/docker/build.go:131	retry/ожидание в подключении к buildkitd	медленный или ложно-красный тест	перепроверить сценарий при изменении логики подключения

Mutation evidence

Каждый раунд: применить → task --yes server:test:unit в отдельной копии → git checkout -- → git diff --quiet HEAD (все раунды чисто). Платформа: macOS/arm64.

№	Mutation	Test that must fail	Result
M1	path_release.go:133 → req.Storage	TestReleaseTaskDoesNotUseRequestStorage (require.NotPanics)	Killed
M2	path_release.go:155 → req.Storage	тот же	Survived на macOS — по построению (GetSettings выходит до чтения вне linux/amd64/cgo)
M3	path_release.go:172 → Storage: req.Storage	паника в горутине сборки	Killed — смерть тестового бинаря, RC=201
M4	path_publish.go:153 → req.Storage	TestPublishTaskDoesNotUseRequestStorage	Killed
M5	release: storage = &logical.InmemStorage{} в начале замыкания	TestReleaseTask… (require.Positive)	Killed
M6	publish: то же	TestPublishTask…	Killed
M7	path_publish.go:197 → req.Storage.Put	нет теста, достигающего строки	Survived (предсказано; строка PR не менялась)

Not verified

• task server:build (CGO ELF-подпись, только Linux); убийство M2 на linux/amd64 (docker-хост недоступен); шаг prettier; реальная builtin-маршрутизация Vault → trdl (семантика роутера проверена по vault/router.go@v1.19.0, форк Stronghold не сверялся); стендовые замеры из комментариев PR; task e2e:test:e2e.

---

Author's response

Принято в db8bc63: compile-time проверки интерфейсов у обоих дублёров; exec.LookPath("git") с явным сообщением в начале initGitRepository.

Оставлено: helpers_test.go — в корневом пакете server/ такого файла нет, все моки живут в backend_test.go; выносить один хелпер отдельно от них значит завести второе место для одного и того же, решение за maintainer'ом. path_publish.go:197 — строка PR не менялась и уже читала хранилище задачи; сценарий до CommitStaged требует дублёра publisher.RepositoryInterface, это отдельная работа над MockedPublisher. Риск 1 (recover во вложенной горутине сборки) — существующий код, вне диффа, стоит отдельного PR.