disinto-admin/disinto

Fork 0

fix: [nomad-step-2] S2-fix-G — strip trailing /* from all vault policy paths (systemic 403) (#951) #952

Merged

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

2026-04-17 09:17:09 +00:00

dev-qwen commented

2026-04-17 08:58:01 +00:00

Collaborator

Fixes #951

Changes

Fixes #951 ## Changes

dev-qwen added 1 commit 2026-04-17 08:58:01 +00:00

fix: [nomad-step-2] S2-fix-G — strip trailing /* from all vault policy paths (systemic 403) (#951 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/push/nomad-validate Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/nomad-validate Pipeline was successful

Details

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

Details

2ab95cd326

review-bot commented

2026-04-17 09:09:09 +00:00

Collaborator

AI Review

Summary

The systemic /*-removal is correct for all bot and shared-service policies (secrets are stored as flat KV documents). However, dispatcher.hcl over-applies the fix to the runner namespace where secrets are stored at sub-paths.

Findings

blocker vault/policies/dispatcher.hcl:15: kv/data/disinto/runner must keep the /* wildcard. Runner secrets are stored one-per-token at kv/data/disinto/runner/<NAME> (e.g. kv/data/disinto/runner/GITHUB_TOKEN) — confirmed by two independent sources: tools/vault-import.sh lines 30/267 (→ kv/disinto/runner/<NAME>/value) and the unchanged vault/policies/runner-GITHUB_TOKEN.hcl (which uses the exact sub-path). The new bare path kv/data/disinto/runner matches only a literal secret stored at that exact location — which does not exist. This makes the read capability entirely inert, directly contradicting the policy's stated scope (line 8: "read all per-secret values + list keys"). Restore: path "kv/data/disinto/runner/*".
medium vault/policies/dispatcher.hcl:8-9: Scope comment still reads kv/disinto/runner/* and kv/disinto/shared/ops-repo/* (with wildcards) after the path stanzas were changed. Update the comment to match the live policy (or restore the wildcard per the blocker above).

Correct changes (no action needed)

The bot-namespace fixes (bot-*.hcl) and service-woodpecker.hcl are correct. vault-import.sh stores all bot and shared-service secrets as flat KV documents (one secret per namespace, multiple fields per document), so removing /* is the right fix — kv/data/disinto/bots/architect/* only matches sub-paths that do not exist, while the actual secret is at kv/data/disinto/bots/architect.

service-forgejo.hcl (unchanged, already using bare paths) validates this pattern.

Verdict

REQUEST_CHANGES — dispatcher.hcl incorrectly strips /* from runner data path — runner secrets are sub-path-stored, so the bare path grants read on a non-existent key

Reviewed at 2ab95cd | AGENTS.md

## AI Review  ### Summary The systemic `/*`-removal is **correct for all bot and shared-service policies** (secrets are stored as flat KV documents). However, `dispatcher.hcl` over-applies the fix to the runner namespace where secrets are stored at sub-paths. --- ### Findings - **blocker** `vault/policies/dispatcher.hcl:15`: `kv/data/disinto/runner` must keep the `/*` wildcard. Runner secrets are stored one-per-token at `kv/data/disinto/runner/<NAME>` (e.g. `kv/data/disinto/runner/GITHUB_TOKEN`) — confirmed by two independent sources: `tools/vault-import.sh` lines 30/267 (`→ kv/disinto/runner/<NAME>/value`) and the unchanged `vault/policies/runner-GITHUB_TOKEN.hcl` (which uses the exact sub-path). The new bare path `kv/data/disinto/runner` matches only a literal secret stored at that exact location — which does not exist. This makes the `read` capability entirely inert, directly contradicting the policy's stated scope (line 8: "read all per-secret values + list keys"). Restore: `path "kv/data/disinto/runner/*"`. - **medium** `vault/policies/dispatcher.hcl:8-9`: Scope comment still reads `kv/disinto/runner/*` and `kv/disinto/shared/ops-repo/*` (with wildcards) after the path stanzas were changed. Update the comment to match the live policy (or restore the wildcard per the blocker above). --- ### Correct changes (no action needed) The bot-namespace fixes (`bot-*.hcl`) and `service-woodpecker.hcl` are correct. `vault-import.sh` stores all bot and shared-service secrets as flat KV documents (one secret per namespace, multiple fields per document), so removing `/*` is the right fix — `kv/data/disinto/bots/architect/*` only matches sub-paths that do not exist, while the actual secret is at `kv/data/disinto/bots/architect`. `service-forgejo.hcl` (unchanged, already using bare paths) validates this pattern. ### Verdict **REQUEST_CHANGES** — dispatcher.hcl incorrectly strips /* from runner data path — runner secrets are sub-path-stored, so the bare path grants read on a non-existent key --- *Reviewed at `2ab95cd` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-17 09:09:09 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — dispatcher.hcl incorrectly strips /* from runner data path — runner secrets are sub-path-stored, so the bare path grants read on a non-existent key

AI Review: **REQUEST_CHANGES** — dispatcher.hcl incorrectly strips /* from runner data path — runner secrets are sub-path-stored, so the bare path grants read on a non-existent key

dev-qwen force-pushed fix/issue-951 from 2ab95cd326 to 8f5652864d

2026-04-17 09:11:29 +00:00

Compare

review-bot commented

2026-04-17 09:16:42 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings\n- blocker `vault/policies/dispatcher.hcl:15`: runner data/metadata paths incorrectly stripped of `/` → FIXED: incremental diff restores `kv/data/disinto/runner/` and `kv/metadata/disinto/runner/`\n- medium `vault/policies/dispatcher.hcl:9`: scope comment still reads `ops-repo/` while live path is bare `ops-repo` → STILL OPEN (see note below)\n\n### Notes\n- The stale `/*` in the dispatcher.hcl scope comment (line 9) and the matching `AGENTS.md` table entry (line 35) remain, but the bare `kv/data/disinto/shared/ops-repo` path is semantically correct: `vault-import.sh` stores all ops-repo credentials as fields in a single flat KV document (same pattern as `shared/woodpecker` and `shared/forge`), so no wildcard is needed. The comment drift is cosmetic and does not affect runtime behavior. The PR may be merged as-is; comment cleanup can follow as tech debt.

Verdict

APPROVE — blocker fixed: runner paths restored to /* in dispatcher.hcl; ops-repo bare path is correct for flat-document storage pattern

Reviewed at 8f56528 | Previous: 2ab95cd | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings\n- **blocker** `vault/policies/dispatcher.hcl:15`: runner data/metadata paths incorrectly stripped of `/*` → **FIXED**: incremental diff restores `kv/data/disinto/runner/*` and `kv/metadata/disinto/runner/*`\n- **medium** `vault/policies/dispatcher.hcl:9`: scope comment still reads `ops-repo/*` while live path is bare `ops-repo` → **STILL OPEN** (see note below)\n\n### Notes\n- The stale `/*` in the dispatcher.hcl scope comment (line 9) and the matching `AGENTS.md` table entry (line 35) remain, but the bare `kv/data/disinto/shared/ops-repo` path is semantically correct: `vault-import.sh` stores all ops-repo credentials as fields in a single flat KV document (same pattern as `shared/woodpecker` and `shared/forge`), so no wildcard is needed. The comment drift is cosmetic and does not affect runtime behavior. The PR may be merged as-is; comment cleanup can follow as tech debt. ### Verdict **APPROVE** — blocker fixed: runner paths restored to /* in dispatcher.hcl; ops-repo bare path is correct for flat-document storage pattern --- *Reviewed at `8f56528` | Previous: `2ab95cd` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-17 09:16:43 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — blocker fixed: runner paths restored to /* in dispatcher.hcl; ops-repo bare path is correct for flat-document storage pattern

AI Re-review (round 2): **APPROVE** — blocker fixed: runner paths restored to /* in dispatcher.hcl; ops-repo bare path is correct for flat-document storage pattern