feat(templates): multiple var groups per template

[fiftin]

No description provided.

[cursor]

Stale comment

Security review (PR #3796)

Outcome: No medium, high, or critical vulnerabilities identified in the added/modified code paths after tracing attacker-controlled input (template JSON from authenticated project users) to sinks (SQL exec/selectAll, GetEnvironment, task merge).

Checks performed

• SQL: UpdateTemplateEnvironments / GetTemplateEnvironments / GetEnvironmentRefs use bound parameters; no string concatenation of user input into queries.
• Cross-project variable groups: GetEnvironment(projectID, envID) filters by project_id, so IDs from other projects do not load data at runtime.
• BoltDB: UpdateTemplateEnvironments is a no-op; EnvironmentIDs still serializes on the template object (same trust model as other template fields).

Slack summary: PR3796 security review: clean — no exploitable issues found; optional hardening would be validating environment_ids belong to the project before insert (defense in depth; FK does not enforce project_id match on environment_id).

Prior automation threads: no thread IDs supplied in this run; used cleanup_previous to reconcile stale automation assessments.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eed6ebfc42

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

[chatgpt-codex-connector]

Prevent cascading deletes of linked template environments

This foreign key uses on delete cascade, so deleting an environment can silently remove rows from project__template_environment even when templates still depend on that variable group. The environment delete flow currently depends on FK failures to surface an in-use error, so this change allows deletion to proceed and strips template configuration instead of blocking with an explicit “in use” failure.

Useful? React with 👍 / 👎.

[cursor]

Stale comment

Security review (PR 3796)

Outcome: No medium, high, or critical security issues were identified in the added or modified code paths after tracing inputs to sinks.

Checks performed

• SQL: GetTemplateEnvironments / UpdateTemplateEnvironments and the GetEnvironmentRefs extension use bound parameters; no string concatenation of user data into queries.
• Cross-project / IDOR on variable groups: TaskRunner.loadEnvironments loads additional groups via GetEnvironment(projectID, envID), which applies project_id scoping (getObject / makeObjectsQuery). Foreign keys on project__template_environment tie environment_id to project__environment, so inserts cannot point at another project’s environments without DB tampering.
• Web UI: New fields use v-autocomplete / {{ }} interpolation; no new v-html or unescaped sinks in the diff.

Prior threads (non-security): Earlier comments about backup omitting environment_ids and ON DELETE CASCADE stripping junction rows are data integrity / ops concerns, not attacker-controlled confidentiality or integrity breaks under normal auth boundaries.

---

Slack-style summary: PR 3796 security pass — no med+ vulns; SQL parameterized; env loads project-scoped; UI changes no new XSS sinks. Prior backup/FK notes are integrity not exploit chain.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR 3796)

Outcome: One medium-confidence issue remains on the BoltDB code path. The SQL path uses parameterized queries and FK constraints on project__template_environment, and GetEnvironment scopes by project_id, so cross-project environment ID injection from environment_ids does not appear exploitable there.

Finding: BoltDB’s generic referrer detection (getReferredValue / isObjectReferredBy) keys off the db:"..." tag suffix environment_id. EnvironmentIDs is tagged db:"-", so templates linked only via this slice are invisible to GetEnvironmentRefs and isObjectInUse checks that scan templates for environment_id. That can allow deleting a variable group that templates still depend on (dangling references / failed tasks) and stale refs in the API—an integrity / availability issue for Bolt deployments, not present on SQL where the junction table is queried explicitly.

No separate high/critical injection or authz bypass was identified in the reviewed diff.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Medium — BoltDB referrer / lifecycle gap

EnvironmentIDs uses db:"-", so getReferredValue(EnvironmentProps, template) never sees this field (db/bolt/BoltDb.go). GetEnvironmentRefs and isObjectInUse(..., EnvironmentProps, ..., TemplateProps) therefore miss templates that only reference environments through environment_ids. On SQL, GetEnvironmentRefs correctly joins project__template_environment; on Bolt, an operator can delete a variable group still in use, breaking tasks and undermining refs-based workflows.

Impact: Data integrity / availability for BoltDB-backed installs; misleading empty refs.

[cursor]

Stale comment

Security review (PR 3796)

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

Checks performed

• Template environment_ids / junction table: UpdateTemplateEnvironments uses parameterized inserts; FKs tie environment_id to project__environment. TaskRunner.loadEnvironments loads each group via GetEnvironment(projectID, envID), which scopes by project, so attacker-chosen IDs from another project do not resolve and do not merge secrets.
• Merge / JSON: Merging variable-group JSON/ENV/secrets mirrors prior single-environment behavior; no new injection sinks beyond existing unmarshaling of stored config.
• Migration PreApply: Dynamic drop foreign key uses a name from INFORMATION_SCHEMA (not user input).
• BoltDB: UpdateTemplateEnvironments is a documented no-op; lists persist on the template object as before.

Prior threads: Previous automation findings were re-evaluated against the current diff; nothing meeting the bar for re-reporting with a concrete exploit path remained. This review supersedes earlier automation assessments (cleanup_previous).

No inline comments filed — no findings at the requested severity.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR #3796)

Outcome: No medium, high, or critical vulnerabilities were identified in the added or modified code paths for this change set.

What was reviewed

• SQL: UpdateTemplateEnvironments / junction table inserts use bound parameters; migration FKs constrain environment_id to project__environment.
• TaskRunner.loadEnvironments: merges JSON/ENV/secrets from environments already loaded via GetEnvironment(projectID, envID), which scopes by project_id like the previous single-environment_id path.
• API: templates still bind from authenticated project context; environment_ids are integers with DB-enforced referential integrity on SQL backends.
• BoltDB: UpdateTemplateEnvironments is a documented no-op (list stored on the template object); behavior is unchanged from prior Bolt template persistence for env linkage.

Prior automation threads: Cleaned up so this assessment is the active one. No prior findings were re-validated as still applicable with a concrete exploit path in this diff.

If you want a follow-up hardening (defense in depth, not rated as a vuln here), consider validating that every environment_id belongs to template.ProjectID before insert so misconfigured clients get a clear error instead of relying on FK failure messages.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR #3796 — multiple variable groups per template)

Outcome: No medium, high, or critical findings after tracing attacker-controlled environment_ids through UpdateTemplateEnvironments → junction table → TaskRunner.loadEnvironments → GetEnvironment / FillEnvironmentSecrets.

Checks performed

• Injection: UpdateTemplateEnvironments uses bound parameters; no string-built SQL from user input.
• Authz / cross-project: GetObject / GetEnvironment constrain by project_id; a forged environment_id from another project yields ErrNotFound at task run, not merged data from that project.
• Deserialization: Merged JSON/ENV unmarshaling matches prior single-environment behavior; empty template JSON handling is a safe guard.
• BoltDB: bolt:"include" only affects serialization of EnvironmentIDs; UpdateTemplateEnvironments remains a no-op as before for Bolt.

Note (below reporting threshold): GetTemplateEnvironments orders by environment_id, so merge precedence may not match UI selection order when IDs are not monotonic with selection — integrity/UX, not a demonstrated privilege boundary.

No inline comments filed because nothing met the medium+ bar with a credible exploit path introduced by this diff.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (automation)

Outcome: No medium, high, or critical vulnerabilities were identified in the added or modified code paths for this change set.

What was reviewed

• API / persistence: environment_ids is bound from JSON into []int and written with parameterized INSERT/DELETE in UpdateTemplateEnvironments; GetEnvironment remains scoped by project_id, so cross-project variable group attachment is blocked by FK + object lookup (same model as the prior single environment_id).
• Task execution: loadEnvironments merges JSON/ENV/secrets only from environments resolved in the same project; no new sinks (SQL/command) from merged content beyond existing task behavior.
• MySQL migration: CONSTRAINT_NAME is read from INFORMATION_SCHEMA for the local project__template table, not from user input; fmt.Sprintf builds the ALTER TABLE … DROP FOREIGN KEY statement with that identifier (acceptable in this context).

Prior threads: No open automation threads were available in this run to resolve individually; cleanup_previous was used so only this assessment remains visible.

---

Slack-ready summary (copy/paste): PR 3796 security review: clean — no medium+ findings. Multi env IDs use parameterized SQL and project-scoped fetches; merge logic does not introduce a new exploitable boundary vs. single env.

No inline comments filed because no high-confidence issues met the reporting bar.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR 3796)

Scope: Multiple variable groups per template (SQL junction project__template_environment, TaskRunner.loadEnvironments merge, API/UI environment_ids, backup/restore/export).

Prior threads: Cleaned up via automation cleanup so only this assessment remains visible.

Verdict: No medium, high, or critical issues identified in the added/changed code paths.

Checks performed

• SQL injection: New queries use bound parameters (? / squirrel); no string-concatenated user input into SQL.
• Authz / cross-project variable groups: GetEnvironment loads by id and project_id (SqlDbConnection.GetObject). Foreign keys do not enforce environment.project_id == template.project_id, but a mismatched environment_id yields ErrNotFound at task run time rather than merging another tenant’s secrets.
• Task runner merge: Merges JSON/ENV/secrets from GetEnvironment results only; duplicate IDs in the list are skipped; empty JSON is handled safely in populateTaskEnvironment.

Residual / out of scope: Same-project users who can edit templates could already attach secrets to tasks; multiple groups does not introduce a new trust boundary beyond existing template permissions.

---

Slack (paste): PR3796 security review: clean — no medium+ findings. Multi-env merge uses parameterized SQL and project-scoped GetEnvironment; no new injection or authz bypass identified in the diff.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR #3796)

Outcome: No medium, high, or critical vulnerabilities identified in the added or changed code paths after tracing attacker-controlled input to sinks.

What was reviewed

• API → persistence: environment_ids from JSON binds into parameterized insert/delete for project__template_environment (db/sql/template.go). No string concatenation of IDs into SQL.
• Task execution: loadEnvironments loads each ID via GetEnvironment(projectID, envID), which constrains rows by project_id and id (db/sql/SqlDb.go GetObject). Merging JSON/ENV/secrets extends existing semantics (same trust boundary as a single environment).
• Cross-project linkage: Foreign keys on project__template_environment require environment_id to reference project__environment; template rows are project-scoped, so arbitrary foreign environment IDs should not attach.
• Migration helper: MySQL PreApply uses fmt.Sprintf with CONSTRAINT_NAME from INFORMATION_SCHEMA; that name is not attacker-controlled in normal deployments (DB metadata). Treated as low confidence for injection and out of scope for medium+ per review criteria.

Prior automation threads: Cleaned up via cleanup_previous so this assessment is the single active security review state.

No inline comments filed because no finding met the bar for reportable severity with a plausible exploit path.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Stale comment

Security review (PR 3796 — multiple variable groups per template)

Scope: Added/modified paths for environment_ids, junction table project__template_environment, TaskRunner.loadEnvironments, API/docs/Dredd hooks, backup/restore, and UI.

Tracing: Template create/update binds db.Template from JSON (AddTemplate / UpdateTemplate). UpdateTemplateEnvironments uses parameterized delete/insert with project_id, template_id, and env_id. Migration v2.18.4.sql adds FKs so rows reference project__environment in the same project. At run time, loadEnvironments calls GetEnvironment(projectID, envID), which enforces project_id in the SQL store (GetObject).

Assessment: No medium / high / critical issues identified with a plausible attacker-controlled sink beyond existing template-edit and project boundaries. Merging multiple groups increases blast radius for users who can already configure templates and environments; that is intentional product behavior, not an authz bypass. Backup/restore maps environment names to IDs with existence checks.

Prior threads: Cleaned via cleanup_previous; no new inline findings (none met the evidence bar).

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Security review (PR #3796)

Scope: Multiple variable groups per template (environment_ids / junction table, merge at task run, UI, backup/restore).

Assessment: No medium, high, or critical issues were identified in the added or modified code paths after tracing attacker-controlled input to sinks.

Checks performed

• SQL: New queries in db/sql/template.go and db/sql/environment.go use bound parameters; no string concatenation of user input into SQL.
• Cross-project / IDOR on environments: GetEnvironment(projectID, environmentID) and the junction writes use the template’s project_id. Foreign keys on project__template_environment tie environment_id to project__environment in the same schema; invalid cross-project pairs should fail at insert or resolve to no row for the runner’s GetEnvironment call.
• Task runner merge (loadEnvironments): Merges JSON/ENV/secrets from environments already loaded under the template’s project; behavior is an extension of the prior single-environment load, not a new trust boundary for unauthenticated callers.
• Front-end: Display uses {{ }} interpolation (Vue escapes HTML); no new v-html or unsafe sinks in the diff.

Residual / out of scope: Pre-existing patterns (e.g. task environment merge, template validation depth) were not escalated without a plausible new exploit tied to this PR.

If prior automation threads referenced concerns that are no longer applicable, they should be cleared by this cleanup pass.

  

_{Sent by Cursor Automation: Find vulnerabilities}