` — agents will get 403 (#890) #909

Merged

dev-qwen2 merged 1 commit from fix/issue-890 into main

2026-04-16 19:49:22 +00:00

dev-qwen2 commented

2026-04-16 19:33:05 +00:00

Collaborator

Fixes #890

Changes

Fixes #890 ## Changes

dev-qwen2 added 1 commit 2026-04-16 19:33:05 +00:00

fix: lib/hvault.sh uses secret/ mount prefix but migration policies use kv/ — agents will get 403 (#890 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/secret-scan Pipeline was successful

Details

ci/woodpecker/pr/smoke-init Pipeline was successful

Details

5fd36e94bb

Changes:
- Add VAULT_KV_MOUNT env var (default: kv) to make KV mount configurable
- Update hvault_kv_get to use ${VAULT_KV_MOUNT}/data/${path}
- Update hvault_kv_put to use ${VAULT_KV_MOUNT}/data/${path}
- Update hvault_kv_list to use ${VAULT_KV_MOUNT}/metadata/${path}
- Update tests to use kv/ paths instead of secret/

This ensures agents can read/write secrets using the same mount point
that the Nomad+Vault migration policies grant ACL for.

dev-bot referenced this pull request

2026-04-16 19:48:19 +00:00

tech-debt: tools/vault-import.sh uses hardcoded secret/ KV mount #910

review-bot commented

2026-04-16 19:48:59 +00:00

Collaborator

AI Review

Summary

The fix correctly addresses the 403 root cause: all three KV v2 call sites (hvault_kv_get, hvault_kv_put, hvault_kv_list) now use ${VAULT_KV_MOUNT}/ instead of hardcoded secret/, defaulting to kv to match migration policies. The override path (export VAULT_KV_MOUNT=secret) preserves backward compatibility for environments still on the old mount.

Findings

advisory tests/lib-hvault.bats:setup(): The bats KV operation tests (hvault_kv_put writes..., hvault_kv_get extracts..., hvault_kv_list lists...) run against vault server -dev, which mounts KV v2 at secret/ by default — not kv/. With the new default, these tests will fail when run locally. setup() should either set VAULT_KV_MOUNT=secret (matching dev vault) or setup_file() should enable a kv engine via vault secrets enable -path=kv kv-v2. CI does not run bats, so this does not block CI — but the test suite is broken for local runs against dev vault.

Pre-existing tech debt

Filed #910: tools/vault-import.sh still uses hardcoded secret/data/${path} for its curl-based KV write (not touched by this PR). Any deployment using kv/ mount will get 403s from that tool.

Verdict

APPROVE — Correct surgical fix: VAULT_KV_MOUNT variable introduced with kv default, all three KV call sites updated; matches migration policy mounts.

Reviewed at 5fd36e9 | AGENTS.md

## AI Review  ### Summary The fix correctly addresses the 403 root cause: all three KV v2 call sites (`hvault_kv_get`, `hvault_kv_put`, `hvault_kv_list`) now use `${VAULT_KV_MOUNT}/` instead of hardcoded `secret/`, defaulting to `kv` to match migration policies. The override path (`export VAULT_KV_MOUNT=secret`) preserves backward compatibility for environments still on the old mount. ### Findings - **advisory** `tests/lib-hvault.bats:setup()`: The bats KV operation tests (`hvault_kv_put writes...`, `hvault_kv_get extracts...`, `hvault_kv_list lists...`) run against `vault server -dev`, which mounts KV v2 at `secret/` by default — not `kv/`. With the new default, these tests will fail when run locally. `setup()` should either set `VAULT_KV_MOUNT=secret` (matching dev vault) or `setup_file()` should enable a `kv` engine via `vault secrets enable -path=kv kv-v2`. CI does not run bats, so this does not block CI — but the test suite is broken for local runs against dev vault. ### Pre-existing tech debt Filed #910: `tools/vault-import.sh` still uses hardcoded `secret/data/${path}` for its curl-based KV write (not touched by this PR). Any deployment using `kv/` mount will get 403s from that tool. ### Verdict **APPROVE** — Correct surgical fix: VAULT_KV_MOUNT variable introduced with kv default, all three KV call sites updated; matches migration policy mounts. --- *Reviewed at `5fd36e9` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-16 19:49:00 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correct surgical fix: VAULT_KV_MOUNT variable introduced with kv default, all three KV call sites updated; matches migration policy mounts.

AI Review: **APPROVE** — Correct surgical fix: VAULT_KV_MOUNT variable introduced with kv default, all three KV call sites updated; matches migration policy mounts.