Skip to content

fix(db): SQL CreateTemplate must return persisted vault IDs#3865

Open
cursor[bot] wants to merge 1 commit into
developfrom
cursor/critical-correctness-bugs-6275
Open

fix(db): SQL CreateTemplate must return persisted vault IDs#3865
cursor[bot] wants to merge 1 commit into
developfrom
cursor/critical-correctness-bugs-6275

Conversation

@cursor
Copy link
Copy Markdown

@cursor cursor Bot commented May 18, 2026

Bug and impact

For the SQL store, CreateTemplate called FillTemplate on a zero-valued template (project/template ID 0), then discarded that result and returned the original request body with newTemplate.ID set. UpdateTemplateVaults inserts new vault rows with real primary keys, but those IDs were never reflected in the API response—clients received vault.id == 0 for newly created vaults. That breaks any follow-up operations that rely on returned IDs and misleads UIs or automation.

Root cause

After insert and UpdateTemplateEnvironments, the code filled a throwaway struct, then overwrote it with newTemplate = template, undoing any DB-backed fields FillTemplate would have loaded if IDs had been set correctly.

Fix

Set newTemplate from the input template, assign newTemplate.ID to the insert ID, then run FillTemplate once so vaults and template–environment links are reloaded from the database (matching the BoltDB path, which already returns the stored object and fills it).

Validation

  • go test ./db/sql/... ./db/... -short

No new integration test was added: the repository does not ship an in-memory SQL CreateTemplate harness; the change is a straightforward ordering fix aligned with FillTemplate’s contract.

Open in Web View Automation 

CreateTemplate called FillTemplate on a zero ID then discarded the
result, so API responses kept vault_id 0 for rows inserted in
UpdateTemplateVaults. Populate the template ID first and return
FillTemplate output so clients get persisted vault and environment IDs.

Co-authored-by: Denis Gukov <fiftin@outlook.com>
@fiftin fiftin marked this pull request as ready for review May 18, 2026 13:41
@fiftin fiftin added the P1 label May 18, 2026
Copy link
Copy Markdown
Author

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Security review (PR #3865)

Outcome: No medium, high, or critical vulnerabilities were identified in the added or modified code.

What changed: CreateTemplate now assigns newTemplate from the input template, sets newTemplate.ID to the insert id, and only then calls db.FillTemplate. That matches how FillTemplate works: it loads vaults, environment links, and related state using template.ProjectID and template.ID. Previously FillTemplate ran against a zero-valued struct (including ID == 0), so hydration was wrong or ineffective; the fix restores correct server-side hydration after create. SQL remains parameterized; no new trust boundary or injection surface was introduced.

Prior automation threads: Stale threads from earlier runs are cleared via cleanup_previous. No open findings required re-posting with new evidence.


Slack summary: PR 3865 security review complete — clean. Change is a DB-layer correctness fix (hydrate template after assign persisted ID); no exploitable authz/authn bypass, injection, or secret exposure identified in the diff.

Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants