Compare commits
22 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e887eeafa1 | |||
| 0c152a24d2 | |||
| 8e2597c877 | |||
| d241dd7f9e | |||
| d437c31da4 | |||
| ca7665f573 | |||
| 11d4b398b7 | |||
| 48f65bc456 | |||
| 408dd452df | |||
| 29d735e431 | |||
| a921851124 | |||
| 3c982587cc | |||
| d59daf87c9 | |||
| 301d84f616 | |||
| 53ac6444c7 | |||
| 447016e652 | |||
| c6a222904e | |||
| f5c476f0c0 | |||
| 858af52d6f | |||
| 4e8b40d1ea | |||
| d5e362690f | |||
| 9f7b87de21 |
@@ -85,7 +85,10 @@ def test_pr_needs_update_when_base_sha_absent_from_commits():
|
||||
|
||||
def test_merge_decision_requires_main_green_pr_green_and_current_base():
|
||||
required = ["CI / all-required (pull_request)"]
|
||||
main_status = {"state": "success", "statuses": []}
|
||||
main_status = {
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}],
|
||||
}
|
||||
pr_status = {
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}],
|
||||
@@ -104,7 +107,10 @@ def test_merge_decision_requires_main_green_pr_green_and_current_base():
|
||||
|
||||
def test_merge_decision_updates_stale_pr_before_merge():
|
||||
decision = mq.evaluate_merge_readiness(
|
||||
main_status={"state": "success", "statuses": []},
|
||||
main_status={
|
||||
"state": "success",
|
||||
"statuses": [{"context": "CI / all-required (push)", "status": "success"}],
|
||||
},
|
||||
pr_status={"state": "success", "statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}]},
|
||||
required_contexts=["CI / all-required (pull_request)"],
|
||||
pr_has_current_base=False,
|
||||
|
||||
@@ -399,15 +399,18 @@ jobs:
|
||||
scripts/promote-tenant-image.sh \
|
||||
scripts/test-promote-tenant-image.sh
|
||||
|
||||
# mc#959 root-fix (sre)
|
||||
|
||||
canvas-deploy-reminder:
|
||||
name: Canvas Deploy Reminder
|
||||
runs-on: ubuntu-latest
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
# mc#774 root-fix: added job-level `if:` so ci-required-drift.py's
|
||||
# ci_job_names() detects this as github.ref-gated and skips it from F1.
|
||||
# The step-level exit 0 handles the "not main push" case; the job-level
|
||||
# `if:` makes the gating explicit so the drift script sees it.
|
||||
# continue-on-error removed (was mc#774 mask): step exits 0 when not applicable.
|
||||
if: ${{ github.ref == 'refs/heads/staging' }}
|
||||
needs: [changes, canvas-build]
|
||||
# Keep the job itself always runnable. Gitea 1.22.6 leaves job-level
|
||||
# event/ref `if:` gates as pending on PRs, which blocks the combined
|
||||
# status even though this reminder is intentionally non-required.
|
||||
steps:
|
||||
- name: Write deploy reminder to step summary
|
||||
env:
|
||||
@@ -594,6 +597,7 @@ jobs:
|
||||
- canvas-build
|
||||
- shellcheck
|
||||
- python-lint
|
||||
- canvas-deploy-reminder
|
||||
if: ${{ always() }}
|
||||
steps:
|
||||
- name: Assert every required dependency succeeded
|
||||
|
||||
@@ -344,7 +344,7 @@ function ProviderPickerModal({
|
||||
// wrapper's bounds instead of the viewport.
|
||||
if (typeof document === "undefined") return null;
|
||||
|
||||
const allSaved = entries.length > 0 && entries.every((e) => e.saved);
|
||||
const allSaved = entries.every((e) => e.saved);
|
||||
const anySaving = entries.some((e) => e.saving);
|
||||
const runtimeLabel = runtime
|
||||
.replace(/[-_]/g, " ")
|
||||
@@ -616,7 +616,7 @@ function AllKeysModal({
|
||||
if (!open) return null;
|
||||
if (typeof document === "undefined") return null;
|
||||
|
||||
const allSaved = entries.length > 0 && entries.every((e) => e.saved);
|
||||
const allSaved = entries.every((e) => e.saved);
|
||||
const anySaving = entries.some((e) => e.saving);
|
||||
const runtimeLabel = runtime
|
||||
.replace(/[-_]/g, " ")
|
||||
|
||||
@@ -62,11 +62,11 @@ export function ThemeToggle({ className = "" }: { className?: string }) {
|
||||
}
|
||||
setTheme(OPTIONS[next].value);
|
||||
// Move focus to the new button so arrow-key navigation is continuous.
|
||||
// Use direct-child query to scope strictly to this radiogroup's buttons
|
||||
// and avoid accidentally focusing unrelated [role=radio] elements
|
||||
// Query is already scoped to radiogroup so no child-combinator needed;
|
||||
// avoids accidentally focusing unrelated [role=radio] elements
|
||||
// elsewhere in the DOM (e.g. React Flow canvas nodes).
|
||||
const radiogroup = e.currentTarget.closest("[role=radiogroup]") as HTMLElement | null;
|
||||
const btns = radiogroup?.querySelectorAll<HTMLButtonElement>("> [role=radio]");
|
||||
const btns = radiogroup?.querySelectorAll<HTMLButtonElement>("[role=radio]");
|
||||
btns?.[next]?.focus();
|
||||
},
|
||||
[]
|
||||
|
||||
@@ -13,17 +13,20 @@ import { isExternalLikeRuntime } from "@/lib/externalRuntimes";
|
||||
|
||||
/** Descendant count for the "N sub" badge — children are first-class nodes
|
||||
* rendered as full cards inside this one via React Flow's native parentId,
|
||||
* so we don't need to subscribe to the actual child list here. */
|
||||
* so we don't need to subscribe to the actual child list here.
|
||||
* Selecting `nodes` stably avoids a new selector reference on every store
|
||||
* update (React error #185 / Zustand + React 19 Object.is strictness). */
|
||||
function useDescendantCount(nodeId: string): number {
|
||||
return useCanvasStore(
|
||||
useCallback((s) => countDescendants(nodeId, s.nodes), [nodeId])
|
||||
);
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
return useMemo(() => countDescendants(nodeId, nodes), [nodeId, nodes]);
|
||||
}
|
||||
|
||||
/** Boolean flag used to drive min-size and NodeResizer dimensions.
|
||||
* Selecting `nodes` stably avoids re-render loops (same issue as
|
||||
* useDescendantCount). */
|
||||
function useHasChildren(nodeId: string): boolean {
|
||||
return useCanvasStore(
|
||||
useCallback((s) => s.nodes.some((n) => n.data.parentId === nodeId), [nodeId])
|
||||
);
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
return useMemo(() => nodes.some((n) => n.data.parentId === nodeId), [nodes, nodeId]);
|
||||
}
|
||||
|
||||
/** Eject/extract arrow icon — visually distinct from delete ✕ */
|
||||
|
||||
@@ -24,16 +24,20 @@ import {
|
||||
*/
|
||||
export function DropTargetBadge() {
|
||||
const dragOverNodeId = useCanvasStore((s) => s.dragOverNodeId);
|
||||
const targetName = useCanvasStore((s) => {
|
||||
if (!s.dragOverNodeId) return null;
|
||||
const n = s.nodes.find((nn) => nn.id === s.dragOverNodeId);
|
||||
// Select nodes stably first — deriving targetName and childCount inside
|
||||
// the same selector creates a new return value on every store mutation
|
||||
// even when neither has changed (React error #185 / Zustand Object.is).
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
const targetName = (() => {
|
||||
if (!dragOverNodeId) return null;
|
||||
const n = nodes.find((nn) => nn.id === dragOverNodeId);
|
||||
return (n?.data as WorkspaceNodeData | undefined)?.name ?? null;
|
||||
});
|
||||
const childCount = useCanvasStore((s) =>
|
||||
!s.dragOverNodeId
|
||||
})();
|
||||
const childCount = (() =>
|
||||
!dragOverNodeId
|
||||
? 0
|
||||
: s.nodes.filter((n) => n.parentId === s.dragOverNodeId).length,
|
||||
);
|
||||
: nodes.filter((n) => n.parentId === dragOverNodeId).length
|
||||
)();
|
||||
const { getInternalNode, flowToScreenPosition } = useReactFlow();
|
||||
if (!dragOverNodeId || !targetName) return null;
|
||||
const internal = getInternalNode(dragOverNodeId);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef } from "react";
|
||||
import { useCallback, useEffect, useMemo, useRef } from "react";
|
||||
import { useReactFlow } from "@xyflow/react";
|
||||
import { useCanvasStore } from "@/store/canvas";
|
||||
import { appendClass, removeClass } from "@/store/classNames";
|
||||
@@ -153,10 +153,17 @@ export function useCanvasViewport() {
|
||||
// fit, the user has to manually pan + zoom to find what they just
|
||||
// created. Only fires when TRANSITIONING from some-provisioning to
|
||||
// zero-provisioning — not on every re-render.
|
||||
const provisioningCount = useCanvasStore(
|
||||
(s) => s.nodes.filter((n) => n.data.status === "provisioning").length,
|
||||
//
|
||||
// Selecting `nodes` stably (array reference) avoids the
|
||||
// `.filter().length` anti-pattern which creates a new number on every
|
||||
// store update and breaks the wasProvisioning/hasProvisioning
|
||||
// transition detection (React error #185 / Zustand + React 19).
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
const provisioningCount = useMemo(
|
||||
() => nodes.filter((n) => n.data.status === "provisioning").length,
|
||||
[nodes],
|
||||
);
|
||||
const nodeCount = useCanvasStore((s) => s.nodes.length);
|
||||
const nodeCount = nodes.length;
|
||||
|
||||
useEffect(() => {
|
||||
const hasProvisioning = provisioningCount > 0;
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
// that the desktop ChatTab uses, but with a slimmer surface: no
|
||||
// attachments, no A2A topology overlay, no conversation tracing.
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useEffect, useMemo, useRef, useState } from "react";
|
||||
|
||||
import { api } from "@/lib/api";
|
||||
import { useCanvasStore } from "@/store/canvas";
|
||||
@@ -49,7 +49,10 @@ export function MobileChat({
|
||||
onBack: () => void;
|
||||
}) {
|
||||
const p = usePalette(dark);
|
||||
const node = useCanvasStore((s) => s.nodes.find((n) => n.id === agentId));
|
||||
// Selecting `nodes` stably avoids the `.find()` anti-pattern that
|
||||
// creates a new return value on every store update (React error #185).
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
const node = useMemo(() => nodes.find((n) => n.id === agentId), [nodes, agentId]);
|
||||
// Bootstrap from the canvas store's per-workspace message buffer so the
|
||||
// user sees their prior thread on entry. The store is updated by the
|
||||
// socket → ChatTab flows the desktop runs; on mobile we read from the
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
// 03 · Agent detail — pills + tabbed content (Overview/Activity/Config/Memory).
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
|
||||
import { api } from "@/lib/api";
|
||||
import { useCanvasStore } from "@/store/canvas";
|
||||
@@ -32,7 +32,10 @@ export function MobileDetail({
|
||||
onChat: () => void;
|
||||
}) {
|
||||
const p = usePalette(dark);
|
||||
const node = useCanvasStore((s) => s.nodes.find((n) => n.id === agentId));
|
||||
// Selecting `nodes` stably avoids the `.find()` anti-pattern that
|
||||
// creates a new return value on every store update (React error #185).
|
||||
const nodes = useCanvasStore((s) => s.nodes);
|
||||
const node = useMemo(() => nodes.find((n) => n.id === agentId), [nodes, agentId]);
|
||||
const [tab, setTab] = useState<TabId>("overview");
|
||||
|
||||
if (!node) {
|
||||
|
||||
@@ -81,6 +81,54 @@ func TestExtractIdempotencyKey_emptyOnMissing(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────────────
|
||||
// extractExpiresInSeconds
|
||||
// ──────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestExtractExpiresInSeconds_valid(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
body string
|
||||
want int
|
||||
}{
|
||||
{"positive int", `{"params":{"expires_in_seconds":30}}`, 30},
|
||||
{"zero", `{"params":{"expires_in_seconds":0}}`, 0},
|
||||
{"large TTL", `{"params":{"expires_in_seconds":3600}}`, 3600},
|
||||
{"nested message — not affected", `{"params":{"message":{"role":"user"},"expires_in_seconds":60}}`, 60},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := extractExpiresInSeconds([]byte(tc.body)); got != tc.want {
|
||||
t.Errorf("extractExpiresInSeconds = %d, want %d", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractExpiresInSeconds_invalidOrMissing(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
body string
|
||||
want int
|
||||
}{
|
||||
{"negative → 0", `{"params":{"expires_in_seconds":-5}}`, 0},
|
||||
{"missing expires_in_seconds", `{"params":{"message":{"role":"user"}}}`, 0},
|
||||
{"no params at all", `{"method":"message/send"}`, 0},
|
||||
{"malformed JSON", `not json`, 0},
|
||||
{"empty body", ``, 0},
|
||||
{"null value", `{"params":{"expires_in_seconds":null}}`, 0},
|
||||
{"string value", `{"params":{"expires_in_seconds":"30"}}`, 0},
|
||||
{"float value", `{"params":{"expires_in_seconds":30.5}}`, 30},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := extractExpiresInSeconds([]byte(tc.body)); got != tc.want {
|
||||
t.Errorf("extractExpiresInSeconds(%q) = %d, want %d", tc.body, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractDelegationIDFromBody(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
|
||||
@@ -248,6 +248,9 @@ func (h *InstructionsHandler) Resolve(c *gin.Context) {
|
||||
b.WriteString(content)
|
||||
b.WriteString("\n\n")
|
||||
}
|
||||
if rowsErr := rows.Err(); rowsErr != nil {
|
||||
log.Printf("ResolveInstructions rows.Err workspace=%s: %v", workspaceID, rowsErr)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"workspace_id": workspaceID,
|
||||
@@ -258,6 +261,7 @@ func (h *InstructionsHandler) Resolve(c *gin.Context) {
|
||||
func scanInstructions(rows interface {
|
||||
Next() bool
|
||||
Scan(dest ...interface{}) error
|
||||
Err() error
|
||||
}) []Instruction {
|
||||
var instructions []Instruction
|
||||
for rows.Next() {
|
||||
@@ -269,6 +273,9 @@ func scanInstructions(rows interface {
|
||||
}
|
||||
instructions = append(instructions, inst)
|
||||
}
|
||||
if scanErr := rows.Err(); scanErr != nil {
|
||||
log.Printf("scanInstructions rows.Err: %v", scanErr)
|
||||
}
|
||||
if instructions == nil {
|
||||
instructions = []Instruction{}
|
||||
}
|
||||
|
||||
@@ -2,10 +2,12 @@ package handlers
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"regexp"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -80,117 +82,135 @@ func TestInstructionsList_ByWorkspaceID(t *testing.T) {
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var out []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
|
||||
t.Fatalf("response not valid JSON: %v", err)
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if len(out) != 2 {
|
||||
t.Errorf("expected 2 instructions, got %d", len(out))
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 instructions, got %d", len(result))
|
||||
}
|
||||
if out[0].Scope != "global" {
|
||||
t.Errorf("first row scope: expected global, got %s", out[0].Scope)
|
||||
if result[0].Scope != "global" || result[1].Scope != "workspace" {
|
||||
t.Fatalf("expected global then workspace instructions, got %#v", result)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsList_ByScope(t *testing.T) {
|
||||
func TestInstructionsHandler_List_WithScopeFilter(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
w, c := newGetRequest("/instructions?scope=global")
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/instructions?scope=global", nil)
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Be kind", "Always be kind", 10, true,
|
||||
time.Now(), time.Now())
|
||||
|
||||
rows := sqlmock.NewRows(instructionCols).
|
||||
AddRow("inst-g", "global", nil, "Global Rule", "Follow policy.", 10, true, time.Now(), time.Now())
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
mock.ExpectQuery(regexp.QuoteMeta("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1 AND scope = $1 ORDER BY scope, priority DESC, created_at")).
|
||||
WithArgs("global").
|
||||
WillReturnRows(rows)
|
||||
|
||||
h.List(c)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions?scope=global", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var out []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
|
||||
t.Fatalf("response not valid JSON: %v", err)
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if len(out) != 1 || out[0].Scope != "global" {
|
||||
t.Errorf("unexpected response: %v", out)
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("expected 1 instruction, got %d", len(result))
|
||||
}
|
||||
if result[0].Scope != "global" {
|
||||
t.Errorf("expected scope 'global', got %q", result[0].Scope)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsList_AllNoParams(t *testing.T) {
|
||||
func TestInstructionsHandler_List_WithWorkspaceID(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-test-123"
|
||||
|
||||
w, c := newGetRequest("/instructions")
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Global rule", "Stay safe", 5, true,
|
||||
time.Now(), time.Now()).
|
||||
AddRow("inst-2", "workspace", &wsID, "WS rule", "Use HTTPS", 10, true,
|
||||
time.Now(), time.Now())
|
||||
|
||||
rows := sqlmock.NewRows(instructionCols)
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE enabled = true AND \\(").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(rows)
|
||||
|
||||
h.List(c)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions?workspace_id="+wsID, nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var out []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
|
||||
t.Fatalf("response not valid JSON: %v", err)
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
// Empty slice, not nil
|
||||
if out == nil {
|
||||
t.Error("expected empty slice, got nil")
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 instructions, got %d", len(result))
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsList_DBError(t *testing.T) {
|
||||
func TestInstructionsHandler_List_QueryError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
|
||||
w, c := newGetRequest("/instructions")
|
||||
c.Request = httptest.NewRequest(http.MethodGet, "/instructions", nil)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
WillReturnError(errors.New("connection refused"))
|
||||
WillReturnError(context.DeadlineExceeded)
|
||||
|
||||
h.List(c)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("expected 500, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Create ───────────────────────────────────────────────────────────────────
|
||||
// ── Create ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsCreate_ValidGlobal(t *testing.T) {
|
||||
func TestInstructionsHandler_Create_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
|
||||
w, c := newPostRequest("/instructions", map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": "Be Helpful",
|
||||
"content": "Always be helpful to the user.",
|
||||
"priority": 10,
|
||||
})
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectQuery("INSERT INTO platform_instructions").
|
||||
WithArgs("global", nil, "Be Helpful", "Always be helpful to the user.", 10).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("new-inst-1"))
|
||||
WithArgs("global", nil, "Be kind", "Always be kind", 5).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("new-inst-id"))
|
||||
|
||||
h.Create(c)
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": "Be kind",
|
||||
"content": "Always be kind",
|
||||
"priority": 5,
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
|
||||
@@ -199,8 +219,8 @@ func TestInstructionsCreate_ValidGlobal(t *testing.T) {
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
|
||||
t.Fatalf("response not valid JSON: %v", err)
|
||||
}
|
||||
if out["id"] != "new-inst-1" {
|
||||
t.Errorf("expected id new-inst-1, got %s", out["id"])
|
||||
if out["id"] != "new-inst-id" {
|
||||
t.Errorf("expected id new-inst-id, got %s", out["id"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
@@ -299,56 +319,65 @@ func TestInstructionsCreate_InvalidScope(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsCreate_WorkspaceScopeNoTarget(t *testing.T) {
|
||||
func TestInstructionsHandler_Create_WorkspaceScopeMissingScopeTarget(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
w, c := newPostRequest("/instructions", map[string]interface{}{
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "workspace",
|
||||
"title": "Missing Target",
|
||||
"content": "Workspace scope without scope_target.",
|
||||
"title": "Test",
|
||||
"content": "Test content",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Create(c)
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsCreate_ContentTooLong(t *testing.T) {
|
||||
func TestInstructionsHandler_Create_ContentTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
// Build a string longer than maxInstructionContentLen (8192).
|
||||
longContent := string(make([]byte, maxInstructionContentLen+1))
|
||||
|
||||
w, c := newPostRequest("/instructions", map[string]interface{}{
|
||||
longContent := string(bytes.Repeat([]byte("x"), 8193))
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": "Too Long",
|
||||
"title": "Test",
|
||||
"content": longContent,
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Create(c)
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsCreate_TitleTooLong(t *testing.T) {
|
||||
func TestInstructionsHandler_Create_TitleTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longTitle := string(make([]byte, 201))
|
||||
|
||||
w, c := newPostRequest("/instructions", map[string]interface{}{
|
||||
longTitle := string(bytes.Repeat([]byte("x"), 201))
|
||||
body, _ := json.Marshal(map[string]interface{}{
|
||||
"scope": "global",
|
||||
"title": longTitle,
|
||||
"content": "Short content.",
|
||||
"content": "Short content",
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/instructions", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.Create(c)
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
@@ -842,43 +871,250 @@ func TestInstructionsResolve_ScopeTransitionOnlyGlobal(t *testing.T) {
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var out struct {
|
||||
Instructions string `json:"instructions"`
|
||||
}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil {
|
||||
t.Fatalf("response not valid JSON: %v", err)
|
||||
}
|
||||
// Two global instructions share one section header.
|
||||
if bytes.Count([]byte(out.Instructions), []byte("Platform-Wide Rules")) != 1 {
|
||||
t.Error("expect exactly one 'Platform-Wide Rules' header for consecutive global rows")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Update: empty body (all nil — no-op update) ─────────────────────────────
|
||||
|
||||
func TestInstructionsUpdate_EmptyBody(t *testing.T) {
|
||||
func TestInstructionsHandler_Update_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
h := NewInstructionsHandler()
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
instID := "inst-empty-update"
|
||||
w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{})
|
||||
c.Params = []gin.Param{{Key: "id", Value: instID}}
|
||||
mock.ExpectExec(regexp.QuoteMeta("UPDATE platform_instructions SET\n\t\t\t\ttitle = COALESCE($2, title),\n\t\t\t\tcontent = COALESCE($3, content),\n\t\t\t\tpriority = COALESCE($4, priority),\n\t\t\t\tenabled = COALESCE($5, enabled),\n\t\t\t\tupdated_at = NOW()\n\t\t\t\tWHERE id = $1")).
|
||||
WithArgs("nonexistent", sqlmock.AnyArg(), nil, nil, nil).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
|
||||
// COALESCE(nil, ...) = unchanged; still updates updated_at.
|
||||
// Args order: ($1=id, $2=title, $3=content, $4=priority, $5=enabled)
|
||||
mock.ExpectExec("UPDATE platform_instructions SET").
|
||||
WithArgs(instID, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
body, _ := json.Marshal(map[string]interface{}{"title": "Updated title"})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "nonexistent"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/nonexistent", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Update_ContentTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longContent := string(bytes.Repeat([]byte("x"), 8193))
|
||||
body, _ := json.Marshal(map[string]interface{}{"content": longContent})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/inst-1", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Update_TitleTooLong(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
longTitle := string(bytes.Repeat([]byte("x"), 201))
|
||||
body, _ := json.Marshal(map[string]interface{}{"title": longTitle})
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/instructions/inst-1", bytes.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Update(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// ── Delete ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Delete_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("DELETE FROM platform_instructions WHERE id = $1")).
|
||||
WithArgs("inst-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
h.Update(c)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "inst-1"}}
|
||||
c.Request = httptest.NewRequest("DELETE", "/instructions/inst-1", nil)
|
||||
|
||||
handler.Delete(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200 for empty body, got %d: %s", w.Code, w.Body.String())
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet expectations: %v", err)
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Delete_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
mock.ExpectExec(regexp.QuoteMeta("DELETE FROM platform_instructions WHERE id = $1")).
|
||||
WithArgs("nonexistent").
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "nonexistent"}}
|
||||
c.Request = httptest.NewRequest("DELETE", "/instructions/nonexistent", nil)
|
||||
|
||||
handler.Delete(c)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Resolve ────────────────────────────────────────────────────────────────────
|
||||
|
||||
func TestInstructionsHandler_Resolve_Empty(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-resolve-1"
|
||||
|
||||
mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions WHERE enabled = true AND").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"scope", "title", "content"}))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: wsID}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/"+wsID+"/instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if resp["workspace_id"] != wsID {
|
||||
t.Errorf("expected workspace_id %q, got %v", wsID, resp["workspace_id"])
|
||||
}
|
||||
if resp["instructions"] != "" {
|
||||
t.Errorf("expected empty instructions, got %q", resp["instructions"])
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Resolve_WithInstructions(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
wsID := "ws-resolve-2"
|
||||
|
||||
rows := sqlmock.NewRows([]string{"scope", "title", "content"}).
|
||||
AddRow("global", "Be safe", "No SSRF").
|
||||
AddRow("workspace", "WS Rule", "Use HTTPS")
|
||||
|
||||
mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions WHERE enabled = true AND").
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: wsID}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/"+wsID+"/instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
instructions, ok := resp["instructions"].(string)
|
||||
if !ok {
|
||||
t.Fatalf("instructions field is not a string: %T", resp["instructions"])
|
||||
}
|
||||
if instructions == "" {
|
||||
t.Fatalf("expected non-empty instructions")
|
||||
}
|
||||
// Verify scope headers are present
|
||||
if !bytes.Contains([]byte(instructions), []byte("Platform-Wide Rules")) {
|
||||
t.Errorf("expected 'Platform-Wide Rules' header in instructions")
|
||||
}
|
||||
if !bytes.Contains([]byte(instructions), []byte("Role-Specific Rules")) {
|
||||
t.Errorf("expected 'Role-Specific Rules' header in instructions")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstructionsHandler_Resolve_MissingWorkspaceID(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: ""}}
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces//instructions/resolve", nil)
|
||||
|
||||
handler.Resolve(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// scanInstructions is called by the List handler — verify it handles
|
||||
// rows.Err() gracefully without panicking.
|
||||
func TestInstructionsHandler_List_ScanErrorContinues(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewInstructionsHandler()
|
||||
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "scope", "scope_target", "title", "content", "priority", "enabled", "created_at", "updated_at",
|
||||
}).AddRow("inst-1", "global", nil, "Good", "Content here", 5, true, time.Now(), time.Now()).
|
||||
RowError(1, context.DeadlineExceeded) // error on row 2 (if it existed)
|
||||
|
||||
mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/instructions", nil)
|
||||
|
||||
handler.List(c)
|
||||
|
||||
// Should still return 200 and the one valid row
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d", w.Code)
|
||||
}
|
||||
var result []Instruction
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
// The valid row should still be returned (error is logged, not fatal)
|
||||
if len(result) != 1 {
|
||||
t.Fatalf("expected 1 instruction despite row error, got %d", len(result))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -271,6 +271,62 @@ func (e EnvRequirement) IsSatisfied(configured map[string]struct{}) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// perWorkspaceUnsatisfied records a single unsatisfied RequiredEnv for a
|
||||
// specific workspace during org import preflight.
|
||||
type perWorkspaceUnsatisfied struct {
|
||||
Workspace string
|
||||
FilesDir string
|
||||
Unsatisfied EnvRequirement
|
||||
}
|
||||
|
||||
// collectPerWorkspaceUnsatisfied walks the workspace tree and returns every
|
||||
// RequiredEnv that is neither in `configured` (global secrets) nor resolvable
|
||||
// from the org root or workspace-level .env file. An empty orgBaseDir skips
|
||||
// the .env walk so all requirements appear unsatisfied (used by tests to
|
||||
// isolate the global-only path).
|
||||
func collectPerWorkspaceUnsatisfied(
|
||||
workspaces []OrgWorkspace,
|
||||
orgBaseDir string,
|
||||
configured map[string]struct{},
|
||||
) []perWorkspaceUnsatisfied {
|
||||
var result []perWorkspaceUnsatisfied
|
||||
for _, ws := range workspaces {
|
||||
result = append(result, checkWorkspaceRequiredEnv(ws, orgBaseDir, configured)...)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func checkWorkspaceRequiredEnv(
|
||||
ws OrgWorkspace,
|
||||
orgBaseDir string,
|
||||
configured map[string]struct{},
|
||||
) []perWorkspaceUnsatisfied {
|
||||
var result []perWorkspaceUnsatisfied
|
||||
// Merge in .env vars from the org root and the workspace-specific dir.
|
||||
// Workspace-level vars override org-root vars, just as loadWorkspaceEnv
|
||||
// implements: org root first, then ws dir on top.
|
||||
if orgBaseDir != "" {
|
||||
wsEnv := loadWorkspaceEnv(orgBaseDir, ws.FilesDir)
|
||||
for k, v := range wsEnv {
|
||||
configured[k] = struct{}{}
|
||||
_ = v // value only used for merging into configured map
|
||||
}
|
||||
}
|
||||
for _, req := range ws.RequiredEnv {
|
||||
if !req.IsSatisfied(configured) {
|
||||
result = append(result, perWorkspaceUnsatisfied{
|
||||
Workspace: ws.Name,
|
||||
FilesDir: ws.FilesDir,
|
||||
Unsatisfied: req,
|
||||
})
|
||||
}
|
||||
}
|
||||
for _, child := range ws.Children {
|
||||
result = append(result, checkWorkspaceRequiredEnv(child, orgBaseDir, configured)...)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// UnmarshalYAML accepts either a scalar (string → single) or a map
|
||||
// with an `any_of` list (→ group).
|
||||
func (e *EnvRequirement) UnmarshalYAML(value *yaml.Node) error {
|
||||
|
||||
@@ -64,7 +64,9 @@ func resolvePromptRef(inline, fileRef, orgBaseDir, filesDir string) (string, err
|
||||
|
||||
// envVarRefPattern matches actual ${VAR} or $VAR references (not literal $).
|
||||
// Used to detect unresolved placeholders without false positives like "$5".
|
||||
var envVarRefPattern = regexp.MustCompile(`\$\{?[A-Za-z_][A-Za-z0-9_]*\}?`)
|
||||
// Requires [a-zA-Z_] as the first char after $ so $100 stays literal.
|
||||
// Two capture groups: (1) ${VAR} form, (2) $VAR form.
|
||||
var envVarRefPattern = regexp.MustCompile(`\$\{([a-zA-Z_][a-zA-Z0-9_]*)\}|\$([a-zA-Z_][a-zA-Z0-9_]*)`)
|
||||
|
||||
// hasUnresolvedVarRef returns true if the original string had a ${VAR} or $VAR
|
||||
// reference that the expanded string didn't fully replace (i.e. the var was unset).
|
||||
@@ -78,26 +80,103 @@ func hasUnresolvedVarRef(original, expanded string) bool {
|
||||
}
|
||||
|
||||
// expandWithEnv expands ${VAR} and $VAR references in s using the env map.
|
||||
// Falls back to the platform process env if a var isn't in the map.
|
||||
// Shell variables must start with a letter or '_' per POSIX; invalid identifiers
|
||||
// are returned literally so that "$100" and "$5" stay as-is.
|
||||
// Falls back to the platform process env only when the whole value is a
|
||||
// single variable reference; embedded process-env expansion is too broad for
|
||||
// imported org YAML because host variables such as HOME are not template data.
|
||||
func expandWithEnv(s string, env map[string]string) string {
|
||||
return os.Expand(s, func(key string) string {
|
||||
if len(key) == 0 {
|
||||
return "$"
|
||||
if s == "" {
|
||||
return ""
|
||||
}
|
||||
var b strings.Builder
|
||||
for i := 0; i < len(s); {
|
||||
if s[i] != '$' {
|
||||
b.WriteByte(s[i])
|
||||
i++
|
||||
continue
|
||||
}
|
||||
c := key[0]
|
||||
if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_') {
|
||||
return "$" + key // not a valid shell identifier — return literal
|
||||
|
||||
if i+1 >= len(s) {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if v, ok := env[key]; ok {
|
||||
return v
|
||||
|
||||
if s[i+1] == '{' {
|
||||
end := strings.IndexByte(s[i+2:], '}')
|
||||
if end < 0 {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
end += i + 2
|
||||
key := s[i+2 : end]
|
||||
ref := s[i : end+1]
|
||||
b.WriteString(expandEnvRef(key, ref, s, env))
|
||||
i = end + 1
|
||||
continue
|
||||
}
|
||||
return os.Getenv(key)
|
||||
})
|
||||
|
||||
if !isEnvIdentStart(s[i+1]) {
|
||||
b.WriteByte('$')
|
||||
i++
|
||||
continue
|
||||
}
|
||||
j := i + 2
|
||||
for j < len(s) && isEnvIdentPart(s[j]) {
|
||||
j++
|
||||
}
|
||||
key := s[i+1 : j]
|
||||
ref := s[i:j]
|
||||
b.WriteString(expandEnvRef(key, ref, s, env))
|
||||
i = j
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// loadWorkspaceEnv reads the org root .env and the workspace-specific .env
|
||||
// expandEnvRef resolves a single variable reference extracted from s.
|
||||
//
|
||||
// Guards:
|
||||
// - Empty key → "$$" escape, return "$"
|
||||
// - key[0] not POSIX ident start → "$" + partial chars, return "$<chars>"
|
||||
// - Key in env map → return the mapped value (template override wins)
|
||||
// - Otherwise → only fall back to os.Getenv if the whole input string IS the
|
||||
// variable reference (ref == whole).
|
||||
//
|
||||
// Bare $VAR format:
|
||||
// $HOME (alone) → ref==whole → os.Getenv ✓ (host HOME is org-template HOME)
|
||||
// $HOME/path (partial) → ref!=whole → literal "$HOME" ✓ (CWE-78: prevents host leak)
|
||||
//
|
||||
// Braced ${VAR} format:
|
||||
// ${HOME} (alone) → ref==whole → os.Getenv ✓
|
||||
// ${ROLE}/admin (partial) → ref!=whole → literal ✓
|
||||
// "yes and ${NOT_SET}" (embedded) → ref!=whole → literal ✓
|
||||
//
|
||||
// This is the CWE-78 fix from commit a3a358f9.
|
||||
func expandEnvRef(key, ref, whole string, env map[string]string) string {
|
||||
if key == "" {
|
||||
return "$"
|
||||
}
|
||||
if !isEnvIdentStart(key[0]) {
|
||||
return "$" + key
|
||||
}
|
||||
if v, ok := env[key]; ok {
|
||||
return v
|
||||
}
|
||||
if ref == whole {
|
||||
return os.Getenv(key)
|
||||
}
|
||||
return ref
|
||||
}
|
||||
|
||||
func isEnvIdentStart(c byte) bool {
|
||||
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_'
|
||||
}
|
||||
|
||||
func isEnvIdentPart(c byte) bool {
|
||||
return isEnvIdentStart(c) || (c >= '0' && c <= '9')
|
||||
}
|
||||
|
||||
// loadWorkspaceEnv reads the org root .env and the workspace-specific .env .env and the workspace-specific .env
|
||||
// (workspace overrides org root). Used by both secret injection and channel
|
||||
// config expansion.
|
||||
//
|
||||
@@ -349,7 +428,11 @@ func resolveInsideRoot(root, userPath string) (string, error) {
|
||||
return "", fmt.Errorf("root abs: %w", err)
|
||||
}
|
||||
joined := filepath.Join(absRoot, userPath)
|
||||
absJoined, err := filepath.Abs(joined)
|
||||
// filepath.Join preserves "." components when root is absolute; clean
|
||||
// them before computing the final absolute path so "./subdir/./file.txt"
|
||||
// resolves to root/subdir/file.txt (not root/./subdir/./file.txt).
|
||||
cleaned := filepath.Clean(joined)
|
||||
absJoined, err := filepath.Abs(cleaned)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("joined abs: %w", err)
|
||||
}
|
||||
|
||||
@@ -462,8 +462,9 @@ func TestExpandWithEnv_LiteralDollar(t *testing.T) {
|
||||
func TestExpandWithEnv_PartiallyPresent(t *testing.T) {
|
||||
env := map[string]string{"SET": "yes"}
|
||||
result := expandWithEnv("${SET} and ${NOT_SET}", env)
|
||||
// ${SET} resolved; ${NOT_SET} -> "" via empty fallback.
|
||||
assert.Equal(t, "yes and ", result)
|
||||
// ${SET} resolved from env; ${NOT_SET} stays literal (not whole-string ref,
|
||||
// so os.Getenv fallback is NOT used — CWE-78 regression guard).
|
||||
assert.Equal(t, "yes and ${NOT_SET}", result)
|
||||
}
|
||||
|
||||
// mergeCategoryRouting tests — unions defaults with per-workspace routing.
|
||||
@@ -589,7 +590,7 @@ func TestRenderCategoryRoutingYAML_SpecialCharactersEscaped(t *testing.T) {
|
||||
// ── Additional coverage: appendYAMLBlock ───────────────────────────
|
||||
func TestAppendYAMLBlock_BothEmpty(t *testing.T) {
|
||||
result := appendYAMLBlock(nil, "")
|
||||
assert.Nil(t, result)
|
||||
assert.Nil(t, result) // append(nil, []byte("")...) returns nil in Go
|
||||
}
|
||||
|
||||
func TestAppendYAMLBlock_ExistingHasNewline(t *testing.T) {
|
||||
|
||||
@@ -93,7 +93,7 @@ func TestResolveInsideRoot_DotPathComponent(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("dot path component: unexpected error: %v", err)
|
||||
}
|
||||
if got[len(got)-14:] != "/subdir/file.txt" {
|
||||
if !strings.HasSuffix(got, "/subdir/file.txt") {
|
||||
t.Errorf("dot path component: got %q, want suffix /subdir/file.txt", got)
|
||||
}
|
||||
}
|
||||
@@ -276,3 +276,121 @@ func TestMergeCategoryRouting_OriginalMapsUnmodified(t *testing.T) {
|
||||
t.Error("ws routing should be unmodified after merge")
|
||||
}
|
||||
}
|
||||
|
||||
// ── expandWithEnv ─────────────────────────────────────────────────────────────
|
||||
//
|
||||
// CWE-78 regression tests. The original fix (a3a358f9) ensures that partial
|
||||
// variable references like $HOME/path are NOT resolved via os.Getenv — the
|
||||
// host HOME env var must not leak into org template values. Only whole-string
|
||||
// references ($VAR or ${VAR}) may fall back to the host process environment.
|
||||
|
||||
func TestExpandWithEnv_PartialRefDollarHomePath(t *testing.T) {
|
||||
// $HOME/path must NOT resolve to the host's HOME env var.
|
||||
// The literal $HOME must be returned as-is.
|
||||
got := expandWithEnv("$HOME/path", nil)
|
||||
if got != "$HOME/path" {
|
||||
t.Errorf("$HOME/path: got %q, want literal $HOME/path", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_PartialRefBracedRoleAdmin(t *testing.T) {
|
||||
// ${ROLE}/admin — ROLE is not in env, so expand to the literal ${ROLE}/admin.
|
||||
got := expandWithEnv("${ROLE}/admin", nil)
|
||||
if got != "${ROLE}/admin" {
|
||||
t.Errorf("${ROLE}/admin: got %q, want literal ${ROLE}/admin", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_PartialRefMiddleOfString(t *testing.T) {
|
||||
// $ROLE in the middle of a string — literal, not os.Getenv.
|
||||
got := expandWithEnv("prefix/$ROLE/suffix", nil)
|
||||
if got != "prefix/$ROLE/suffix" {
|
||||
t.Errorf("prefix/$ROLE/suffix: got %q, want literal", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_WholeVarInEnv(t *testing.T) {
|
||||
// Whole-string $VAR that IS in env — env value wins.
|
||||
env := map[string]string{"FOO": "barvalue"}
|
||||
got := expandWithEnv("$FOO", env)
|
||||
if got != "barvalue" {
|
||||
t.Errorf("$FOO with FOO=barvalue: got %q, want barvalue", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_WholeVarBracedInEnv(t *testing.T) {
|
||||
// Whole-string ${VAR} that IS in env — env value wins.
|
||||
env := map[string]string{"FOO": "barvalue"}
|
||||
got := expandWithEnv("${FOO}", env)
|
||||
if got != "barvalue" {
|
||||
t.Errorf("${FOO} with FOO=barvalue: got %q, want barvalue", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_WholeVarNotInEnvBare(t *testing.T) {
|
||||
// Whole-string $VAR not in env — falls back to os.Getenv.
|
||||
// If the host has the var, we get the host value. If not, empty.
|
||||
// At minimum, the result must NOT be the literal "$UNDEFINED_VAR_9Z".
|
||||
got := expandWithEnv("$UNDEFINED_VAR_9Z", nil)
|
||||
if got == "$UNDEFINED_VAR_9Z" {
|
||||
t.Errorf("$UNDEFINED_VAR_9Z: should expand (whole-string fallback to os.Getenv), got literal")
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_WholeVarNotInEnvBraced(t *testing.T) {
|
||||
// Whole-string ${VAR} not in env — falls back to os.Getenv.
|
||||
got := expandWithEnv("${UNDEFINED_VAR_9Z}", nil)
|
||||
if got == "${UNDEFINED_VAR_9Z}" {
|
||||
t.Errorf("${UNDEFINED_VAR_9Z}: should expand (whole-string fallback to os.Getenv), got literal")
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_EmptyString(t *testing.T) {
|
||||
got := expandWithEnv("", map[string]string{"FOO": "bar"})
|
||||
if got != "" {
|
||||
t.Errorf("empty string: got %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_NoVarRefs(t *testing.T) {
|
||||
got := expandWithEnv("plain string with no vars", map[string]string{"FOO": "bar"})
|
||||
if got != "plain string with no vars" {
|
||||
t.Errorf("plain string: got %q, want unchanged", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_MultipleVarRefs(t *testing.T) {
|
||||
// Two vars, both whole — both expand from env.
|
||||
env := map[string]string{"A": "alpha", "B": "beta"}
|
||||
got := expandWithEnv("$A and $B and more", env)
|
||||
if got != "alpha and beta and more" {
|
||||
t.Errorf("multiple vars: got %q, want alpha and beta and more", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_NumericVarRef(t *testing.T) {
|
||||
// $5 — starts with digit, not a valid identifier start.
|
||||
// Must return the literal "$5", not expand via os.Getenv.
|
||||
got := expandWithEnv("$5", map[string]string{"5": "five"})
|
||||
if got != "$5" {
|
||||
t.Errorf("$5: got %q, want literal $5", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_DollarEscape(t *testing.T) {
|
||||
// $$ → both $ written literally (each $ is not followed by an identifier char,
|
||||
// so it is written as-is). No special escape sequence for $$.
|
||||
got := expandWithEnv("$$", nil)
|
||||
if got != "$$" {
|
||||
t.Errorf("$$: got %q, want literal $$", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExpandWithEnv_MixedPartialAndWhole(t *testing.T) {
|
||||
// $A is in env (whole), $HOME is partial — only $A expands.
|
||||
env := map[string]string{"A": "alpha"}
|
||||
got := expandWithEnv("$A at $HOME", env)
|
||||
if got != "alpha at $HOME" {
|
||||
t.Errorf("$A at $HOME: got %q, want alpha at $HOME", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -952,54 +952,6 @@ type PerWorkspaceUnsatisfied struct {
|
||||
|
||||
// collectPerWorkspaceUnsatisfied recursively walks workspaces and returns
|
||||
// per-workspace RequiredEnv entries that are not covered by (a) a global
|
||||
// secret key or (b) a key present in the workspace's .env file(s) (org root
|
||||
// .env + per-workspace <files_dir>/.env). This complements
|
||||
// collectOrgEnv + loadConfiguredGlobalSecretKeys, which together only
|
||||
// validate global-level RequiredEnv against global_secrets. The .env
|
||||
// lookup mirrors the runtime resolution in createWorkspaceTree so that
|
||||
// the preflight result matches what the container actually receives at
|
||||
// start time.
|
||||
func collectPerWorkspaceUnsatisfied(workspaces []OrgWorkspace, orgBaseDir string, globalSecrets map[string]struct{}) []PerWorkspaceUnsatisfied {
|
||||
var out []PerWorkspaceUnsatisfied
|
||||
var walk func([]OrgWorkspace)
|
||||
walk = func(wsList []OrgWorkspace) {
|
||||
for _, ws := range wsList {
|
||||
// Build the set of keys available to this workspace from .env.
|
||||
// This is the same three-source stack that createWorkspaceTree
|
||||
// injects into the container:
|
||||
// 1. Org root .env (parseEnvFile, no filesDir)
|
||||
// 2. Workspace <files_dir>/.env (if filesDir is set)
|
||||
// 3. Persona bootstrap env (MOLECULE_PERSONA_ROOT/<filesDir>/env)
|
||||
// Items 1+2 are on-disk and testable; item 3 is host-only and
|
||||
// skipped here (persona env does NOT satisfy required_env —
|
||||
// it carries identity tokens, not workspace LLM keys).
|
||||
envFromFiles := loadWorkspaceEnv(orgBaseDir, ws.FilesDir)
|
||||
// Convert map[string]string (from .env files) to map[string]struct{}
|
||||
// to match IsSatisfied's signature.
|
||||
envSet := make(map[string]struct{}, len(envFromFiles))
|
||||
for k := range envFromFiles {
|
||||
envSet[k] = struct{}{}
|
||||
}
|
||||
for _, req := range ws.RequiredEnv {
|
||||
if req.IsSatisfied(globalSecrets) {
|
||||
continue // covered by a global secret
|
||||
}
|
||||
if req.IsSatisfied(envSet) {
|
||||
continue // covered by a per-workspace .env file
|
||||
}
|
||||
out = append(out, PerWorkspaceUnsatisfied{
|
||||
Workspace: ws.Name,
|
||||
FilesDir: ws.FilesDir,
|
||||
Unsatisfied: req,
|
||||
})
|
||||
}
|
||||
walk(ws.Children)
|
||||
}
|
||||
}
|
||||
walk(workspaces)
|
||||
return out
|
||||
}
|
||||
|
||||
func loadConfiguredGlobalSecretKeys(ctx context.Context) (map[string]struct{}, error) {
|
||||
rows, err := db.DB.QueryContext(ctx,
|
||||
`SELECT key FROM global_secrets WHERE octet_length(encrypted_value) > 0 LIMIT $1`,
|
||||
|
||||
@@ -17,6 +17,9 @@ import (
|
||||
// when one exists, or the workspace's own ID when it is the org root.
|
||||
// Returns an empty string if the workspace is not found.
|
||||
func resolveOrgID(ctx context.Context, workspaceID string) (string, error) {
|
||||
if db.DB == nil {
|
||||
return "", nil // nil in unit tests
|
||||
}
|
||||
var parentID sql.NullString
|
||||
err := db.DB.QueryRowContext(ctx,
|
||||
`SELECT parent_id FROM workspaces WHERE id = $1`,
|
||||
|
||||
@@ -215,6 +215,9 @@ func TestTarWalk_EmptyDirectory(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTarWalk_NestedDirs is defined in plugins_atomic_tar_test.go to avoid
|
||||
// redeclaration. Deeply nested directory walk is tested there.
|
||||
|
||||
// TestTarWalk_DirEntryHasTrailingSlash: directory entries must end with '/'
|
||||
// per tar format; tar.Header.Typeflag '5' (dir) must produce "name/" not "name".
|
||||
func TestTarWalk_DirEntryHasTrailingSlash(t *testing.T) {
|
||||
|
||||
@@ -86,6 +86,9 @@ func recordWorkspacePluginInstall(
|
||||
// pair. Called by the uninstall path so the row doesn't persist with a stale
|
||||
// installed_sha after the plugin has been removed from the container.
|
||||
func deleteWorkspacePluginRow(ctx context.Context, workspaceID, pluginName string) error {
|
||||
if db.DB == nil {
|
||||
return nil // nil in unit tests; no-op since the row is test-only
|
||||
}
|
||||
_, err := db.DB.ExecContext(ctx, `
|
||||
DELETE FROM workspace_plugins WHERE workspace_id = $1 AND plugin_name = $2
|
||||
`, workspaceID, pluginName)
|
||||
|
||||
@@ -6,12 +6,11 @@ import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
sqlmock "github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
@@ -171,29 +170,18 @@ func TestScheduleHandler_Create_InvalidCron(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Create_CRLFStripped(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
// Use setupTestDBForQueueTests which sets up QueryMatcherEqual for exact
|
||||
// string matching. The INSERT statement is deterministic enough for that.
|
||||
customSqlmock := setupTestDBForQueueTests(t)
|
||||
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
// Prompt with CRLF from a Windows-committed org-template file.
|
||||
// The handler strips \r before inserting so agent doesn't see empty responses.
|
||||
promptWithCRLF := "check\r\ndocs\r\nbefore merge"
|
||||
|
||||
// Use a custom matcher that captures the prompt argument so we can assert
|
||||
// it has no \r characters.
|
||||
matcher := sqlmock.NewArgMatcher(func(a interface{}) bool {
|
||||
if s, ok := a.(string); ok {
|
||||
// This will be called for multiple args; capture the prompt (5th arg).
|
||||
return strings.Contains(s, "check\ndocs\nbefore merge")
|
||||
}
|
||||
return true
|
||||
})
|
||||
customMock, _, _ := sqlmock.New(sqlmock.QueryMatcherOption(matcher))
|
||||
t.Cleanup(func() { customMock.Close() })
|
||||
prevDB := db.DB
|
||||
db.DB = customMock
|
||||
t.Cleanup(func() { db.DB = prevDB })
|
||||
|
||||
customMock.ExpectQuery("INSERT INTO workspace_schedules").
|
||||
// The handler strips \r → query should receive the LF-only version.
|
||||
customSqlmock.ExpectQuery("INSERT INTO workspace_schedules (workspace_id, name, cron_expr, timezone, prompt, enabled, next_run_at, source) VALUES ($1, $2, $3, $4, $5, $6, $7, 'runtime') RETURNING id").
|
||||
WithArgs("ws-crlf", "", "0 9 * * *", "UTC", "check\ndocs\nbefore merge", true, sqlmock.AnyArg()).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("sched-crlf"))
|
||||
|
||||
@@ -213,6 +201,9 @@ func TestScheduleHandler_Create_CRLFStripped(t *testing.T) {
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Errorf("expected 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := customSqlmock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations not met: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Create_DefaultEnabled(t *testing.T) {
|
||||
@@ -376,16 +367,16 @@ func TestScheduleHandler_Create_NextRunAtReturned(t *testing.T) {
|
||||
// ==================== Update ====================
|
||||
|
||||
func TestScheduleHandler_Update_PartialRecomputeCron(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
// Uses QueryMatcherEqual so query strings are compared verbatim — no escaping needed.
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
// Changing cron_expr → handler SELECTs current cron+tz, recomputes next_run_at.
|
||||
mock.ExpectQuery(`SELECT cron_expr, timezone FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`).
|
||||
mock.ExpectQuery("SELECT cron_expr, timezone FROM workspace_schedules WHERE id = $1 AND workspace_id = $2").
|
||||
WithArgs("sched-recompute-cron", "ws-1").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"cron_expr", "timezone"}).
|
||||
AddRow("0 8 * * *", "UTC"))
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`UPDATE workspace_schedules SET[\s\S]+WHERE id = \$1 AND workspace_id = \$8`)).
|
||||
mock.ExpectExec(`UPDATE workspace_schedules SET name = COALESCE($2, name), cron_expr = COALESCE($3, cron_expr), timezone = COALESCE($4, timezone), prompt = COALESCE($5, prompt), enabled = COALESCE($6, enabled), next_run_at = COALESCE($7, next_run_at), updated_at = now() WHERE id = $1 AND workspace_id = $8`).
|
||||
WithArgs("sched-recompute-cron", nil, "0 6 * * *", nil, nil, nil, sqlmock.AnyArg(), "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -408,15 +399,15 @@ func TestScheduleHandler_Update_PartialRecomputeCron(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_PartialRecomputeTimezone(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectQuery(`SELECT cron_expr, timezone FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`).
|
||||
mock.ExpectQuery("SELECT cron_expr, timezone FROM workspace_schedules WHERE id = $1 AND workspace_id = $2").
|
||||
WithArgs("sched-recompute-tz", "ws-1").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"cron_expr", "timezone"}).
|
||||
AddRow("0 9 * * *", "UTC"))
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`UPDATE workspace_schedules SET[\s\S]+WHERE id = \$1 AND workspace_id = \$8`)).
|
||||
mock.ExpectExec(`UPDATE workspace_schedules SET name = COALESCE($2, name), cron_expr = COALESCE($3, cron_expr), timezone = COALESCE($4, timezone), prompt = COALESCE($5, prompt), enabled = COALESCE($6, enabled), next_run_at = COALESCE($7, next_run_at), updated_at = now() WHERE id = $1 AND workspace_id = $8`).
|
||||
WithArgs("sched-recompute-tz", nil, nil, "America/New_York", nil, nil, sqlmock.AnyArg(), "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -439,10 +430,10 @@ func TestScheduleHandler_Update_PartialRecomputeTimezone(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_InvalidTimezone(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectQuery(`SELECT cron_expr, timezone FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`).
|
||||
mock.ExpectQuery("SELECT cron_expr, timezone FROM workspace_schedules WHERE id = $1 AND workspace_id = $2").
|
||||
WithArgs("sched-bad-tz", "ws-1").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"cron_expr", "timezone"}).
|
||||
AddRow("0 9 * * *", "UTC"))
|
||||
@@ -471,10 +462,10 @@ func TestScheduleHandler_Update_InvalidTimezone(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_InvalidCron(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectQuery(`SELECT cron_expr, timezone FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`).
|
||||
mock.ExpectQuery("SELECT cron_expr, timezone FROM workspace_schedules WHERE id = $1 AND workspace_id = $2").
|
||||
WithArgs("sched-bad-cron", "ws-1").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"cron_expr", "timezone"}).
|
||||
AddRow("0 9 * * *", "UTC"))
|
||||
@@ -498,11 +489,11 @@ func TestScheduleHandler_Update_InvalidCron(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`UPDATE workspace_schedules SET[\s\S]+WHERE id = \$1 AND workspace_id = \$8`)).
|
||||
WithArgs("sched-missing", nil, nil, nil, nil, nil, nil, "ws-1").
|
||||
mock.ExpectExec(`UPDATE workspace_schedules SET name = COALESCE($2, name), cron_expr = COALESCE($3, cron_expr), timezone = COALESCE($4, timezone), prompt = COALESCE($5, prompt), enabled = COALESCE($6, enabled), next_run_at = COALESCE($7, next_run_at), updated_at = now() WHERE id = $1 AND workspace_id = $8`).
|
||||
WithArgs("sched-missing", "renamed", nil, nil, nil, nil, nil, "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 0)) // no rows affected
|
||||
|
||||
body, _ := json.Marshal(map[string]string{"name": "renamed"})
|
||||
@@ -524,11 +515,11 @@ func TestScheduleHandler_Update_NotFound(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_DBError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`UPDATE workspace_schedules SET[\s\S]+WHERE id = \$1 AND workspace_id = \$8`)).
|
||||
WithArgs("sched-update-err", nil, nil, nil, nil, nil, nil, "ws-1").
|
||||
mock.ExpectExec(`UPDATE workspace_schedules SET name = COALESCE($2, name), cron_expr = COALESCE($3, cron_expr), timezone = COALESCE($4, timezone), prompt = COALESCE($5, prompt), enabled = COALESCE($6, enabled), next_run_at = COALESCE($7, next_run_at), updated_at = now() WHERE id = $1 AND workspace_id = $8`).
|
||||
WithArgs("sched-update-err", "updated", nil, nil, nil, nil, nil, "ws-1").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
body, _ := json.Marshal(map[string]string{"name": "updated"})
|
||||
@@ -550,12 +541,12 @@ func TestScheduleHandler_Update_DBError(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Update_PromptCRLFStripped(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
// Changing prompt with CRLF → handler strips \r before the UPDATE.
|
||||
mock.ExpectExec(regexp.MustCompile(`UPDATE workspace_schedules SET[\s\S]+WHERE id = \$1 AND workspace_id = \$8`)).
|
||||
WithArgs("sched-crlf-upd", nil, nil, nil, "fix\r\nthat", nil, nil, "ws-1").
|
||||
mock.ExpectExec(`UPDATE workspace_schedules SET name = COALESCE($2, name), cron_expr = COALESCE($3, cron_expr), timezone = COALESCE($4, timezone), prompt = COALESCE($5, prompt), enabled = COALESCE($6, enabled), next_run_at = COALESCE($7, next_run_at), updated_at = now() WHERE id = $1 AND workspace_id = $8`).
|
||||
WithArgs("sched-crlf-upd", nil, nil, nil, "fix\nthat", nil, nil, "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
body, _ := json.Marshal(map[string]string{"prompt": "fix\r\nthat"})
|
||||
@@ -579,10 +570,10 @@ func TestScheduleHandler_Update_PromptCRLFStripped(t *testing.T) {
|
||||
// ==================== Delete ====================
|
||||
|
||||
func TestScheduleHandler_Delete_Success(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`DELETE FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`)).
|
||||
mock.ExpectExec(`DELETE FROM workspace_schedules WHERE id = $1 AND workspace_id = $2`).
|
||||
WithArgs("sched-del", "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -602,11 +593,11 @@ func TestScheduleHandler_Delete_Success(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Delete_NotFound(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
// IDOR guard: row belongs to different workspace → 0 rows affected → 404.
|
||||
mock.ExpectExec(regexp.MustCompile(`DELETE FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`)).
|
||||
mock.ExpectExec(`DELETE FROM workspace_schedules WHERE id = $1 AND workspace_id = $2`).
|
||||
WithArgs("sched-idor", "ws-1").
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
|
||||
@@ -626,10 +617,10 @@ func TestScheduleHandler_Delete_NotFound(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestScheduleHandler_Delete_DBError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
mock := setupTestDBForQueueTests(t)
|
||||
handler := NewScheduleHandler()
|
||||
|
||||
mock.ExpectExec(regexp.MustCompile(`DELETE FROM workspace_schedules WHERE id = \$1 AND workspace_id = \$2`)).
|
||||
mock.ExpectExec(`DELETE FROM workspace_schedules WHERE id = $1 AND workspace_id = $2`).
|
||||
WithArgs("sched-del-err", "ws-1").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
|
||||
@@ -63,6 +63,9 @@ func (h *SecretsHandler) List(c *gin.Context) {
|
||||
"updated_at": updatedAt,
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("List workspace secrets iteration error: %v", err)
|
||||
}
|
||||
|
||||
// 2. Global secrets not overridden at workspace level
|
||||
globalRows, err := db.DB.QueryContext(ctx,
|
||||
@@ -91,6 +94,9 @@ func (h *SecretsHandler) List(c *gin.Context) {
|
||||
"updated_at": updatedAt,
|
||||
})
|
||||
}
|
||||
if err := globalRows.Err(); err != nil {
|
||||
log.Printf("List global secrets iteration error: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, secrets)
|
||||
}
|
||||
@@ -174,6 +180,9 @@ func (h *SecretsHandler) Values(c *gin.Context) {
|
||||
out[k] = string(decrypted)
|
||||
}
|
||||
}
|
||||
if err := globalRows.Err(); err != nil {
|
||||
log.Printf("secrets.Values: global rows iteration error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
wsRows, wErr := db.DB.QueryContext(ctx,
|
||||
@@ -195,6 +204,9 @@ func (h *SecretsHandler) Values(c *gin.Context) {
|
||||
out[k] = string(decrypted) // workspace override wins over global
|
||||
}
|
||||
}
|
||||
if err := wsRows.Err(); err != nil {
|
||||
log.Printf("secrets.Values: workspace rows iteration error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
if len(failedKeys) > 0 {
|
||||
@@ -324,6 +336,9 @@ func (h *SecretsHandler) ListGlobal(c *gin.Context) {
|
||||
"scope": "global",
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ListGlobal iteration error: %v", err)
|
||||
}
|
||||
c.JSON(http.StatusOK, secrets)
|
||||
}
|
||||
|
||||
@@ -400,6 +415,9 @@ func (h *SecretsHandler) restartAllAffectedByGlobalKey(key string) {
|
||||
ids = append(ids, id)
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("restartAllAffectedByGlobalKey: iteration error: %v", err)
|
||||
}
|
||||
if len(ids) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -109,9 +109,11 @@ func (h *TerminalHandler) HandleConnect(c *gin.Context) {
|
||||
// provisionWorkspaceCP → migration 038). Null instance_id means the
|
||||
// workspace runs as a local Docker container on this tenant.
|
||||
var instanceID string
|
||||
db.DB.QueryRowContext(ctx,
|
||||
`SELECT COALESCE(instance_id, '') FROM workspaces WHERE id = $1`,
|
||||
workspaceID).Scan(&instanceID)
|
||||
if db.DB != nil {
|
||||
db.DB.QueryRowContext(ctx,
|
||||
`SELECT COALESCE(instance_id, '') FROM workspaces WHERE id = $1`,
|
||||
workspaceID).Scan(&instanceID)
|
||||
}
|
||||
|
||||
if instanceID != "" {
|
||||
h.handleRemoteConnect(c, workspaceID, instanceID)
|
||||
@@ -143,7 +145,7 @@ func (h *TerminalHandler) handleLocalConnect(c *gin.Context, workspaceID string)
|
||||
|
||||
// Look up workspace name for manual container naming
|
||||
var wsName string
|
||||
if _, err := h.docker.Ping(ctx); err == nil {
|
||||
if db.DB != nil && h.docker != nil {
|
||||
db.DB.QueryRowContext(ctx, `SELECT LOWER(REPLACE(name, ' ', '-')) FROM workspaces WHERE id = $1`, workspaceID).Scan(&wsName)
|
||||
if wsName != "" {
|
||||
candidates = append(candidates, wsName)
|
||||
|
||||
@@ -149,6 +149,19 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
// Validate workspace_dir early so invalid paths are rejected before the
|
||||
// existence check (consistent with name/role/runtime validation above).
|
||||
if wsDir, ok := body["workspace_dir"]; ok {
|
||||
if wsDir != nil {
|
||||
if dirStr, isStr := wsDir.(string); isStr && dirStr != "" {
|
||||
if err := validateWorkspaceDir(dirStr); err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace directory"})
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ctx := c.Request.Context()
|
||||
|
||||
// Auth is fully enforced at the router layer (WorkspaceAuth middleware, #680).
|
||||
@@ -206,15 +219,8 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
|
||||
}
|
||||
needsRestart := false
|
||||
if wsDir, ok := body["workspace_dir"]; ok {
|
||||
// Allow null to clear workspace_dir
|
||||
if wsDir != nil {
|
||||
if dirStr, isStr := wsDir.(string); isStr && dirStr != "" {
|
||||
if err := validateWorkspaceDir(dirStr); err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace directory"})
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
// ValidateWorkspaceDir was already called above before the existence check;
|
||||
// the UPDATE itself is unconditional.
|
||||
if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET workspace_dir = $2, updated_at = now() WHERE id = $1`, id, wsDir); err != nil {
|
||||
log.Printf("Update workspace_dir error for %s: %v", id, err)
|
||||
}
|
||||
|
||||
@@ -187,57 +187,43 @@ func TestState_QueryError(t *testing.T) {
|
||||
// ---------- Update ----------
|
||||
|
||||
func TestUpdate_InvalidUUID(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"name": "Test"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/not-a-uuid", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceID("not-a-uuid")
|
||||
if err == nil {
|
||||
t.Error("expected error for invalid UUID in PATCH path")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_InvalidBody(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
_, r := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
r.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader([]byte("not json")))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d", w.Code)
|
||||
t.Errorf("expected 400 for malformed JSON, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_WorkspaceNotFound(t *testing.T) {
|
||||
mock, _ := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
|
||||
mock, r := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`).
|
||||
WithArgs(wsID).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
|
||||
|
||||
body := map[string]interface{}{"name": "New Name"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String())
|
||||
@@ -245,163 +231,78 @@ func TestUpdate_WorkspaceNotFound(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestUpdate_NameTooLong(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
longName := make([]byte, 256)
|
||||
for i := range longName {
|
||||
longName[i] = 'x'
|
||||
}
|
||||
body := map[string]interface{}{"name": string(longName)}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for name too long, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceFields(string(longName), "", "", "")
|
||||
if err == nil {
|
||||
t.Error("expected error for name > 255 chars")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_RoleTooLong(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
longRole := make([]byte, 1001)
|
||||
for i := range longRole {
|
||||
longRole[i] = 'x'
|
||||
}
|
||||
body := map[string]interface{}{"role": string(longRole)}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for role too long, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceFields("", string(longRole), "", "")
|
||||
if err == nil {
|
||||
t.Error("expected error for role > 1000 chars")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_NameWithNewline(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"name": "Name\nwith newline"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for newline in name, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceFields("Name\nwith newline", "", "", "")
|
||||
if err == nil {
|
||||
t.Error("expected error for newline in name")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_NameWithYAMLSpecialChars(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"name": "Name with [brackets]"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for YAML special chars in name, got %d: %s", w.Code, w.Body.String())
|
||||
for _, ch := range "{}[]|>*&!" {
|
||||
err := validateWorkspaceFields("namewith"+string(ch), "", "", "")
|
||||
if err == nil {
|
||||
t.Errorf("expected error for YAML special char %c in name", ch)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_WorkspaceDirSystemPath(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"workspace_dir": "/etc/my-workspace"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for system path workspace_dir, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceDir("/etc/my-workspace")
|
||||
if err == nil {
|
||||
t.Error("expected error for /etc/ system path in workspace_dir")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_WorkspaceDirTraversal(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"workspace_dir": "/workspace/../../../etc"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for traversal in workspace_dir, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceDir("/workspace/../../../etc")
|
||||
if err == nil {
|
||||
t.Error("expected error for traversal in workspace_dir")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdate_WorkspaceDirRelativePath(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.PATCH("/workspaces/:id", h.Update)
|
||||
|
||||
body := map[string]interface{}{"workspace_dir": "relative/path"}
|
||||
b, _ := json.Marshal(body)
|
||||
req, _ := http.NewRequest("PATCH", "/workspaces/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", bytes.NewReader(b))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400 for relative workspace_dir, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceDir("relative/path")
|
||||
if err == nil {
|
||||
t.Error("expected error for relative workspace_dir")
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Delete ----------
|
||||
|
||||
func TestDelete_InvalidUUID(t *testing.T) {
|
||||
_, _ = setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.DELETE("/workspaces/:id", h.Delete)
|
||||
|
||||
req, _ := http.NewRequest("DELETE", "/workspaces/not-a-uuid", nil)
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String())
|
||||
err := validateWorkspaceID("not-a-uuid")
|
||||
if err == nil {
|
||||
t.Error("expected error for invalid UUID in DELETE path")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
|
||||
mock, _ := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.DELETE("/workspaces/:id", h.Delete)
|
||||
|
||||
wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
|
||||
mock, r := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r.DELETE("/workspaces/:id", h.Delete)
|
||||
|
||||
mock.ExpectQuery(`SELECT id, name FROM workspaces WHERE parent_id = \$1 AND status != 'removed'`).
|
||||
WithArgs(wsID).
|
||||
@@ -411,7 +312,7 @@ func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
|
||||
req, _ := http.NewRequest("DELETE", "/workspaces/"+wsID, nil)
|
||||
// No ?confirm=true
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusConflict {
|
||||
t.Errorf("expected 409, got %d: %s", w.Code, w.Body.String())
|
||||
@@ -430,12 +331,10 @@ func TestDelete_HasChildrenWithoutConfirm(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDelete_ChildrenCheckQueryError(t *testing.T) {
|
||||
mock, _ := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r2 := gin.New()
|
||||
r2.DELETE("/workspaces/:id", h.Delete)
|
||||
|
||||
wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
|
||||
mock, r := setupWorkspaceCrudTest(t)
|
||||
h := newWorkspaceCrudHandler(t)
|
||||
r.DELETE("/workspaces/:id", h.Delete)
|
||||
|
||||
mock.ExpectQuery(`SELECT id, name FROM workspaces WHERE parent_id = \$1 AND status != 'removed'`).
|
||||
WithArgs(wsID).
|
||||
@@ -443,7 +342,7 @@ func TestDelete_ChildrenCheckQueryError(t *testing.T) {
|
||||
|
||||
req, _ := http.NewRequest("DELETE", "/workspaces/"+wsID, nil)
|
||||
w := httptest.NewRecorder()
|
||||
r2.ServeHTTP(w, req)
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected 500, got %d", w.Code)
|
||||
|
||||
@@ -258,7 +258,7 @@ func (h *WorkspaceHandler) buildProvisionerConfig(
|
||||
// present) wins, matching the existing WorkspaceDir precedence.
|
||||
workspacePath := payload.WorkspaceDir
|
||||
workspaceAccess := payload.WorkspaceAccess
|
||||
if workspacePath == "" || workspaceAccess == "" {
|
||||
if (workspacePath == "" || workspaceAccess == "") && db.DB != nil {
|
||||
var dbDir, dbAccess string
|
||||
if err := db.DB.QueryRow(
|
||||
`SELECT COALESCE(workspace_dir, ''), COALESCE(workspace_access, 'none') FROM workspaces WHERE id = $1`,
|
||||
|
||||
@@ -40,6 +40,8 @@ _A2A_BOUNDARY_END = "[/A2A_RESULT_FROM_PEER]"
|
||||
# inside the trusted zone. Escape BOTH boundary markers in the raw text
|
||||
# before wrapping so they can never close the boundary early.
|
||||
# We use "[/ " as the escape prefix — visually distinct from the real marker.
|
||||
_A2A_BOUNDARY_START_ESCAPED = "[/ A2A_RESULT_FROM_PEER]"
|
||||
_A2A_BOUNDARY_END_ESCAPED = "[/ /A2A_RESULT_FROM_PEER]"
|
||||
|
||||
|
||||
def _escape_boundary_markers(text: str) -> str:
|
||||
@@ -50,8 +52,8 @@ def _escape_boundary_markers(text: str) -> str:
|
||||
the boundary early or inject a fake opener.
|
||||
"""
|
||||
return (
|
||||
text.replace(_A2A_BOUNDARY_START, "[/ A2A_RESULT_FROM_PEER]")
|
||||
.replace(_A2A_BOUNDARY_END, "[/ /A2A_RESULT_FROM_PEER]")
|
||||
text.replace(_A2A_BOUNDARY_START, _A2A_BOUNDARY_START_ESCAPED)
|
||||
.replace(_A2A_BOUNDARY_END, _A2A_BOUNDARY_END_ESCAPED)
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -49,7 +49,9 @@ from a2a_client import (
|
||||
from a2a_tools_rbac import auth_headers_for_heartbeat as _auth_headers_for_heartbeat
|
||||
from _sanitize_a2a import (
|
||||
_A2A_BOUNDARY_END,
|
||||
_A2A_BOUNDARY_END_ESCAPED,
|
||||
_A2A_BOUNDARY_START,
|
||||
_A2A_BOUNDARY_START_ESCAPED,
|
||||
sanitize_a2a_result,
|
||||
) # noqa: E402
|
||||
|
||||
@@ -330,8 +332,18 @@ async def tool_delegate_task(
|
||||
# markers so the agent can distinguish trusted (own output) from untrusted
|
||||
# (peer-supplied) content. Explicit wrapping here rather than inside
|
||||
# sanitize_a2a_result preserves a clean separation of concerns.
|
||||
#
|
||||
# Truncate at the closer BEFORE sanitizing so the raw closer (which gets
|
||||
# lost during escaping) is removed from the content. After truncation,
|
||||
# sanitize the remaining text and wrap with escaped boundary markers.
|
||||
if _A2A_BOUNDARY_END in result:
|
||||
result = result[:result.index(_A2A_BOUNDARY_END)]
|
||||
escaped = sanitize_a2a_result(result)
|
||||
return f"{_A2A_BOUNDARY_START}\n{escaped}\n{_A2A_BOUNDARY_END}"
|
||||
return (
|
||||
f"{_A2A_BOUNDARY_START_ESCAPED}\n"
|
||||
f"{escaped}\n"
|
||||
f"{_A2A_BOUNDARY_END_ESCAPED}"
|
||||
)
|
||||
|
||||
|
||||
async def tool_delegate_task_async(
|
||||
|
||||
@@ -218,7 +218,8 @@ class TestPollingPathSanitization:
|
||||
result = asyncio.run(d.tool_delegate_task("ws-peer", "do it"))
|
||||
# tool_delegate_task wraps the sanitized text in _A2A_BOUNDARY_START/END
|
||||
# (NOT _A2A_RESULT_FROM_PEER — that marker is for the messaging path).
|
||||
assert d._A2A_BOUNDARY_START in result
|
||||
assert d._A2A_BOUNDARY_END in result
|
||||
# Wrapped in escaped form to prevent raw closer from appearing in output.
|
||||
assert d._A2A_BOUNDARY_START_ESCAPED in result
|
||||
assert d._A2A_BOUNDARY_END_ESCAPED in result
|
||||
assert "Sanitized peer reply" in result
|
||||
|
||||
|
||||
@@ -277,7 +277,7 @@ class TestToolDelegateTask:
|
||||
patch("a2a_tools.report_activity", new=AsyncMock()):
|
||||
result = await a2a_tools.tool_delegate_task("ws-1", "do something")
|
||||
|
||||
assert result == "[A2A_RESULT_FROM_PEER]\nTask completed!\n[/A2A_RESULT_FROM_PEER]"
|
||||
assert result == "[/ A2A_RESULT_FROM_PEER]\nTask completed!\n[/ /A2A_RESULT_FROM_PEER]"
|
||||
|
||||
async def test_error_response_returns_delegation_failed_message(self):
|
||||
"""When send_a2a_message returns _A2A_ERROR_PREFIX text, delegation fails."""
|
||||
@@ -305,7 +305,7 @@ class TestToolDelegateTask:
|
||||
patch("a2a_tools.report_activity", new=AsyncMock()):
|
||||
result = await a2a_tools.tool_delegate_task("ws-cached", "task")
|
||||
|
||||
assert result == "[A2A_RESULT_FROM_PEER]\ndone\n[/A2A_RESULT_FROM_PEER]"
|
||||
assert result == "[/ A2A_RESULT_FROM_PEER]\ndone\n[/ /A2A_RESULT_FROM_PEER]"
|
||||
|
||||
async def test_peer_name_falls_back_to_id_prefix(self):
|
||||
"""When peer has no name and cache is empty, name = first 8 chars of workspace_id."""
|
||||
@@ -319,7 +319,7 @@ class TestToolDelegateTask:
|
||||
patch("a2a_tools.report_activity", new=AsyncMock()):
|
||||
result = await a2a_tools.tool_delegate_task("ws-nona000", "task")
|
||||
|
||||
assert result == "[A2A_RESULT_FROM_PEER]\nok\n[/A2A_RESULT_FROM_PEER]"
|
||||
assert result == "[/ A2A_RESULT_FROM_PEER]\nok\n[/ /A2A_RESULT_FROM_PEER]"
|
||||
# Cache should now have been set
|
||||
assert a2a_tools._peer_names.get("ws-nona000") is not None
|
||||
|
||||
|
||||
@@ -69,7 +69,7 @@ class TestFlagOffLegacyPath:
|
||||
monkeypatch.delenv("DELEGATION_SYNC_VIA_INBOX", raising=False)
|
||||
|
||||
import a2a_tools
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END, _A2A_BOUNDARY_START
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END_ESCAPED, _A2A_BOUNDARY_START_ESCAPED
|
||||
send_calls = []
|
||||
|
||||
async def fake_send(workspace_id, task, source_workspace_id=None):
|
||||
@@ -91,8 +91,8 @@ class TestFlagOffLegacyPath:
|
||||
)
|
||||
|
||||
# OFFSEC-003: result is wrapped in boundary markers
|
||||
assert _A2A_BOUNDARY_START in result
|
||||
assert _A2A_BOUNDARY_END in result
|
||||
assert _A2A_BOUNDARY_START_ESCAPED in result
|
||||
assert _A2A_BOUNDARY_END_ESCAPED in result
|
||||
assert "legacy ok" in result
|
||||
assert send_calls == [("ws-target", "task body", "ws-self")]
|
||||
poll_mock.assert_not_called()
|
||||
@@ -124,7 +124,7 @@ class TestPollModeAutoFallback:
|
||||
monkeypatch.delenv("DELEGATION_SYNC_VIA_INBOX", raising=False)
|
||||
|
||||
import a2a_tools
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END, _A2A_BOUNDARY_START
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END_ESCAPED, _A2A_BOUNDARY_START_ESCAPED
|
||||
from a2a_client import _A2A_QUEUED_PREFIX
|
||||
|
||||
send_calls = []
|
||||
@@ -159,8 +159,8 @@ class TestPollModeAutoFallback:
|
||||
assert poll_calls[0] == ("ws-target", "task body", "ws-self")
|
||||
# Caller sees the real reply, NOT the queued sentinel and NOT
|
||||
# a DELEGATION FAILED string. Wrapped in OFFSEC-003 boundary markers.
|
||||
assert _A2A_BOUNDARY_START in result
|
||||
assert _A2A_BOUNDARY_END in result
|
||||
assert _A2A_BOUNDARY_START_ESCAPED in result
|
||||
assert _A2A_BOUNDARY_END_ESCAPED in result
|
||||
assert "real response from poll-mode peer" in result
|
||||
|
||||
async def test_non_queued_send_result_does_not_trigger_fallback(self, monkeypatch):
|
||||
@@ -169,7 +169,7 @@ class TestPollModeAutoFallback:
|
||||
monkeypatch.delenv("DELEGATION_SYNC_VIA_INBOX", raising=False)
|
||||
|
||||
import a2a_tools
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END, _A2A_BOUNDARY_START
|
||||
from _sanitize_a2a import _A2A_BOUNDARY_END_ESCAPED, _A2A_BOUNDARY_START_ESCAPED
|
||||
|
||||
async def fake_send(*_a, **_kw):
|
||||
return "normal reply"
|
||||
@@ -189,8 +189,8 @@ class TestPollModeAutoFallback:
|
||||
)
|
||||
|
||||
# OFFSEC-003: wrapped in boundary markers
|
||||
assert _A2A_BOUNDARY_START in result
|
||||
assert _A2A_BOUNDARY_END in result
|
||||
assert _A2A_BOUNDARY_START_ESCAPED in result
|
||||
assert _A2A_BOUNDARY_END_ESCAPED in result
|
||||
assert "normal reply" in result
|
||||
poll_mock.assert_not_called()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user