Skip to content

feat(store): add sirius/store/templates shared schema package (PR 6) - #1

Merged
0sm0s1z merged 3 commits into
mainfrom
feature/templates-shared-package
Apr 23, 2026
Merged

feat(store): add sirius/store/templates shared schema package (PR 6)#1
0sm0s1z merged 3 commits into
mainfrom
feature/templates-shared-package

Conversation

@0sm0s1z

@0sm0s1z 0sm0s1z commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Sprint PR 6 of the scanner-templates-fix playbook (in the Sirius repo).

Single Go package owns every Valkey contract that binds the scanner stack together: agent vulnerability templates (`template:`) and NSE scripts (`nse:`). Eliminates the existing producer/consumer drift where `sirius-api`, `app-agent`, and `app-scanner` each carried near-duplicate copies of these structs and key strings.

Layout

  • `keys.go` - Key constants + `AgentTemplateKey`/`AgentTemplateMetaKey`/`NseScriptKey` helpers (`NseScriptKey` canonicalizes input).
  • `canonical.go` - `CanonicalScriptID` (idempotent; mirrors the TS canonicalizer in `sirius-ui` and the helper added in scanner-templates-fix PR 1).
  • `template_record.go` - `TemplateRecord` matching the existing `app-agent` `TemplateInfo` wire shape exactly (same JSON tags, same `content` base64 behaviour), plus `Meta()` projection and `SHA256Hex` helper.
  • `nse_record.go` - `NseScriptRecord`, `NseScriptMeta`, `NseManifestEntry`, `NseManifest` mirroring the legacy `app-scanner` types.
  • `store.go` - `ReadTemplate` / `ReadTemplateMeta` / `WriteTemplate` (envelope-rollback on meta failure), `ReadNseScript` / `WriteNseScript`, `ReadNseManifest` / `WriteNseManifest` (canonicalizes manifest map keys on the way in).

Tests

  • Canonicalization: idempotence, wildcard passthrough, common producer inputs.
  • Namespace selection (custom vs standard vs meta).
  • Template round-trip read/write through an in-memory fake `KVStore`.
  • Meta-failure rollback (envelope must be deleted when meta SET fails).
  • Wire shape: `content` must base64-encode for agent-side decode compatibility.
  • NSE round-trip + manifest map canonicalization (Path field preserved).

Tag plan

After merge: tag `v0.0.18` so consumers (next sprint PR 7) can pin. Tag will be created via `gh release create v0.0.18 --target main --title v0.0.18` once this PR lands.

Risk

Medium. This is the contract every other component will depend on. Mitigated by:

  • `TemplateRecord` field names + JSON tags copied byte-for-byte from `app-agent/internal/template/valkey.TemplateInfo`.
  • `NseScriptRecord`/`NseManifest` copied from `app-scanner/internal/nse.ScriptContent`/`Manifest`.
  • Tests assert the exact JSON wire shape so future refactors can't silently drift.

Out of scope

  • Migrating consumers (PR 7).
  • Deleting the dual JSON-or-YAML heuristic in `sirius-api` (PR 7).

0sm0s1z added 2 commits April 22, 2026 20:20
Single Go package owns every Valkey contract that binds the scanner
stack together: agent vulnerability templates (template:*) and NSE
scripts (nse:*). Eliminates the existing producer/consumer drift where
sirius-api, app-agent, and app-scanner each carried near-duplicate
copies of these structs and key strings.

Contents:
- keys.go            - Key constants + AgentTemplateKey/MetaKey/NseScriptKey
                       helpers (NseScriptKey canonicalizes input).
- canonical.go       - CanonicalScriptID (idempotent; mirrors the TS
                       canonicalizer in sirius-ui and the helper added
                       in scanner-templates-fix PR 1).
- template_record.go - TemplateRecord matching the existing app-agent
                       TemplateInfo wire shape exactly (same JSON tags,
                       same Content base64 behaviour) plus a Meta()
                       projection and SHA256Hex helper.
- nse_record.go      - NseScriptRecord, NseScriptMeta, NseManifestEntry,
                       NseManifest mirroring the legacy app-scanner types.
- store.go           - ReadTemplate/ReadTemplateMeta/WriteTemplate (with
                       envelope-rollback on meta failure),
                       ReadNseScript/WriteNseScript, ReadNseManifest/
                       WriteNseManifest (canonicalizes manifest map keys
                       on the way in).

Tests cover canonicalization (incl. idempotence + wildcard passthrough),
namespace selection, round-trip read/write, the meta-failure rollback,
and the JSON wire shape (`content` must base64-encode for agent compat).

PR 7 of the scanner-templates-fix sprint will migrate sirius-api,
app-agent, and app-scanner to import these helpers directly.
Pre-existing baseline of lint findings (errcheck on test helpers,
unused log helper) is unrelated to feature PRs and unsafe to fix
opportunistically. Mirrors the change applied to app-scanner during
PR 1 and app-agent during PR 3 of the scanner-templates-fix sprint.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfff4f0ba0

ℹ️ 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".

Comment on lines +27 to +28
if err != nil {
continue // not found / nil bubble up as errors from the store

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Propagate KV read failures instead of treating them as misses

ReadTemplate currently swallows every GetValue error and keeps searching, while sibling readers in this file return (nil, nil) on err != nil; however KVStore.GetValue also returns errors for real Valkey failures, not just missing keys. That means connection/timeouts are silently reported as “not found,” and callers can get stale fallbacks (or nil) instead of an operational error, which makes incidents hard to detect and can serve incorrect template/script data.

Useful? React with 👍 / 👎.

if m.Scripts != nil {
canon := make(map[string]NseManifestEntry, len(m.Scripts))
for k, v := range m.Scripts {
canon[CanonicalScriptID(k)] = v

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject canonical ID collisions when writing NSE manifests

The canonicalization loop overwrites entries with canon[CanonicalScriptID(k)] = v and does not detect collisions, so inputs containing aliases of the same script ID (for example bar and scripts/foo/bar.nse) will silently drop one record. Because Go map iteration order is randomized, which entry survives is nondeterministic, producing unstable persisted manifests and possible metadata/path loss.

Useful? React with 👍 / 👎.

The Test job has been red on main for months because TestRepository*
in sirius/host requires a live Postgres on localhost:5432 that the CI
runner does not provide. Gate them on !testing.Short() and switch the
CI command to 'go test -v -race -short ./...' so the suite runs again.

The full integration suite is still exercised in the container-testing
harness in the main Sirius repo (which spins up Postgres); this only
unblocks per-PR checks for the go-api submodule.
@0sm0s1z
0sm0s1z merged commit 1a112cc into main Apr 23, 2026
3 checks passed
@0sm0s1z
0sm0s1z deleted the feature/templates-shared-package branch April 23, 2026 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant