Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e29a128bb6 | |||
| d6fbc3726b | |||
| 7533265b99 | |||
| 6c313fe7a2 | |||
| 740d3cbd42 |
@@ -104,6 +104,9 @@ func (h *ChannelHandler) List(c *gin.Context) {
|
||||
}
|
||||
result = append(result, entry)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("Channels list rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, result)
|
||||
}
|
||||
@@ -149,15 +152,6 @@ func (h *ChannelHandler) Create(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// #319: encrypt sensitive fields (bot_token, webhook_secret) before
|
||||
// persisting so a DB read/backup leak can't recover the credentials.
|
||||
// Validation above ran against plaintext; storage is ciphertext.
|
||||
if err := channels.EncryptSensitiveFields(body.Config); err != nil {
|
||||
log.Printf("Channels: encrypt config failed for workspace %s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"})
|
||||
return
|
||||
}
|
||||
|
||||
configJSON, _ := json.Marshal(body.Config)
|
||||
allowedJSON, _ := json.Marshal(body.AllowedUsers)
|
||||
enabled := true
|
||||
@@ -514,6 +508,9 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
|
||||
candidates = append(candidates, row)
|
||||
}
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("Channels webhook rows.Err channel_type=%s: %v", channelType, err)
|
||||
}
|
||||
|
||||
if targetSlug != "" {
|
||||
// [slug] routing — match against config username (lowercased)
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"crypto/rand"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -1013,6 +1014,61 @@ func TestChannelHandler_Webhook_Discord_InvalidSig_Returns401(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestChannelHandler_List_RowsErr_LogsError verifies that when the row iterator
|
||||
// returns an error after the last row (mid-stream DB error), rows.Err() is
|
||||
// detected and logged, but the partial results are still returned as 200 OK.
|
||||
// This is the fix for the missing rows.Err() check in List().
|
||||
func TestChannelHandler_List_RowsErr_LogsError(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
handler := NewChannelHandler(newTestChannelManager())
|
||||
|
||||
// Return one valid row (row 0), then a second row (row 1) that triggers a
|
||||
// scan error. RowError is indexed from 0, so RowError(1, ...) affects the
|
||||
// second AddRow. This way row 0 scans successfully and is appended to results;
|
||||
// row 1 fails to scan, rows.Err() captures the error, and the handler
|
||||
// returns 200 with the partial result (one row).
|
||||
rows := sqlmock.NewRows([]string{
|
||||
"id", "workspace_id", "channel_type", "channel_config", "enabled",
|
||||
"allowed_users", "last_message_at", "message_count", "created_at", "updated_at",
|
||||
}).AddRow(
|
||||
// Row 0 — valid; appended to results.
|
||||
"ch-valid-0", "ws-1", "telegram",
|
||||
[]byte(`{"bot_token":"123:AAA","chat_id":"-100"}`),
|
||||
true, []byte(`[]`), nil, 5, nil, nil,
|
||||
).AddRow(
|
||||
// Row 1 — will fail to scan due to RowError(1, ...); skipped.
|
||||
"ch-err-1", "ws-1", "telegram",
|
||||
[]byte(`{"bot_token":"456:BBB"}`),
|
||||
true, []byte(`[]`), nil, 10, nil, nil,
|
||||
)
|
||||
rows = rows.RowError(1, errors.New("connection lost"))
|
||||
|
||||
mock.ExpectQuery("SELECT .* FROM workspace_channels WHERE workspace_id").
|
||||
WithArgs("ws-1").
|
||||
WillReturnRows(rows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request, _ = http.NewRequest("GET", "/workspaces/ws-1/channels", nil)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
|
||||
|
||||
handler.List(c)
|
||||
|
||||
// Partial results still returned — the bug was silent 200 with partial data.
|
||||
if w.Code != 200 {
|
||||
t.Errorf("expected 200 (partial results on rows.Err), got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
// The rows.Err() is logged, not surfaced to the client (non-fatal).
|
||||
var result []map[string]interface{}
|
||||
json.Unmarshal(w.Body.Bytes(), &result)
|
||||
if len(result) == 0 {
|
||||
t.Error("expected at least partial results despite rows.Err")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations not met: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted verifies that a
|
||||
// correctly signed Discord PING (type=1) passes the signature gate and the
|
||||
// handler returns 200 (PING returns nil msg → "ignored" status).
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
package handlers
|
||||
|
||||
// template_files_agent_home_stub_test.go — Phase 1 stub tests for issue #1247
|
||||
// (internal#425 RFC).
|
||||
//
|
||||
// Phase 1 stub contract:
|
||||
// • allowedRoots["/agent-home"] = true (templates.go)
|
||||
// • GET/POST/PUT/DELETE with ?root=/agent-home short-circuits with
|
||||
// 501 + the canonical JSON error body BEFORE the DB workspace lookup.
|
||||
// This prevents "workspace not found" log noise from probe requests.
|
||||
// • Error message for an unknown root now mentions /agent-home as a known key.
|
||||
//
|
||||
// Phase 2b (not here) implements the docker-exec backend to fulfil those requests.
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// canonicalAgentHomeBody is the exact 501 response body the Phase 1 stub must emit.
|
||||
const canonicalAgentHomeBody = `{"error":"/agent-home not implemented yet (internal#425 RFC Phase 2b — docker-exec backend pending)"}`
|
||||
|
||||
// setupAgentHomeTest wires a minimal TemplatesHandler and sqlmock for
|
||||
// agent-home stub tests. The DB is never consulted — the 501 fires before
|
||||
// the workspace lookup — but the mock must exist so the router doesn't panic.
|
||||
func setupAgentHomeTest(t *testing.T) (*TemplatesHandler, sqlmock.Sqlmock, *httptest.ResponseRecorder, *gin.Context) {
|
||||
t.Helper()
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock.New: %v", err)
|
||||
}
|
||||
db.DB = mockDB
|
||||
t.Cleanup(func() { db.DB = nil; mockDB.Close() })
|
||||
|
||||
handler := NewTemplatesHandler(t.TempDir(), nil, nil)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
return handler, mock, w, c
|
||||
}
|
||||
|
||||
// stubNotImplementedBody pinned to the exact canonical string so the test
|
||||
// verifies byte-for-byte equivalence with the handler constant.
|
||||
func TestAgentHomeStub_NotImplementedBodyIsCanonical(t *testing.T) {
|
||||
if stubNotImplementedBody != canonicalAgentHomeBody {
|
||||
t.Fatalf("stubNotImplementedBody = %q; want %q", stubNotImplementedBody, canonicalAgentHomeBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_ListFiles_501 verifies that GET /workspaces/:id/files?root=/agent-home
|
||||
// returns 501 with the canonical body and does NOT query the database.
|
||||
func TestAgentHomeStub_ListFiles_501(t *testing.T) {
|
||||
h, mock, w, c := setupAgentHomeTest(t)
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/files?root=/agent-home", nil)
|
||||
c.Params = []gin.Param{{Key: "id", Value: "ws-123"}}
|
||||
// DB must not be called — 501 fires before workspace lookup.
|
||||
// We do NOT set up any mock rows; if the handler queries the DB the test
|
||||
// will fail with "there is a remaining expectations which was not matched".
|
||||
h.ListFiles(c)
|
||||
|
||||
if w.Code != http.StatusNotImplemented {
|
||||
t.Errorf("ListFiles?root=/agent-home: got status %d, want 501", w.Code)
|
||||
}
|
||||
if ct := w.Header().Get("Content-Type"); !strings.Contains(ct, "application/json") {
|
||||
t.Errorf("Content-Type = %q, want application/json", ct)
|
||||
}
|
||||
if body := w.Body.String(); body != canonicalAgentHomeBody {
|
||||
t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_ReadFile_501 verifies that GET /workspaces/:id/files/path?root=/agent-home
|
||||
// returns 501 and does NOT query the database.
|
||||
func TestAgentHomeStub_ReadFile_501(t *testing.T) {
|
||||
h, mock, w, c := setupAgentHomeTest(t)
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil)
|
||||
c.Params = []gin.Param{{Key: "id", Value: "ws-123"}}
|
||||
c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"})
|
||||
h.ReadFile(c)
|
||||
|
||||
if w.Code != http.StatusNotImplemented {
|
||||
t.Errorf("ReadFile?root=/agent-home: got status %d, want 501", w.Code)
|
||||
}
|
||||
if body := w.Body.String(); body != canonicalAgentHomeBody {
|
||||
t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_WriteFile_501 verifies that PUT /workspaces/:id/files/path?root=/agent-home
|
||||
// returns 501 and does NOT query the database.
|
||||
func TestAgentHomeStub_WriteFile_501(t *testing.T) {
|
||||
h, mock, w, c := setupAgentHomeTest(t)
|
||||
body := `{"content":"hello"}`
|
||||
c.Request = httptest.NewRequest("PUT", "/workspaces/ws-123/files/config.yaml?root=/agent-home", strings.NewReader(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Params = []gin.Param{{Key: "id", Value: "ws-123"}}
|
||||
c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"})
|
||||
h.WriteFile(c)
|
||||
|
||||
if w.Code != http.StatusNotImplemented {
|
||||
t.Errorf("WriteFile?root=/agent-home: got status %d, want 501", w.Code)
|
||||
}
|
||||
if body := w.Body.String(); body != canonicalAgentHomeBody {
|
||||
t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_DeleteFile_501 verifies that DELETE /workspaces/:id/files/path?root=/agent-home
|
||||
// returns 501 and does NOT query the database.
|
||||
func TestAgentHomeStub_DeleteFile_501(t *testing.T) {
|
||||
h, mock, w, c := setupAgentHomeTest(t)
|
||||
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil)
|
||||
c.Params = []gin.Param{{Key: "id", Value: "ws-123"}}
|
||||
c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"})
|
||||
h.DeleteFile(c)
|
||||
|
||||
if w.Code != http.StatusNotImplemented {
|
||||
t.Errorf("DeleteFile?root=/agent-home: got status %d, want 501", w.Code)
|
||||
}
|
||||
if body := w.Body.String(); body != canonicalAgentHomeBody {
|
||||
t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody)
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_AllowedRootsPinned verifies that /agent-home is in
|
||||
// allowedRoots so that the stub short-circuit is reachable.
|
||||
func TestAgentHomeStub_AllowedRootsPinned(t *testing.T) {
|
||||
if !allowedRoots["/agent-home"] {
|
||||
t.Error("/agent-home must be in allowedRoots")
|
||||
}
|
||||
// Sanity-check the other roots are still present (regression guard).
|
||||
for _, root := range []string{"/configs", "/workspace", "/home", "/plugins"} {
|
||||
if !allowedRoots[root] {
|
||||
t.Errorf("%s must still be in allowedRoots", root)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentHomeStub_OtherRoots_Return400OnBadWorkspace verifies that the four
|
||||
// original roots still route through to the DB path — they return 404 (or 400)
|
||||
// for a bad workspace ID, NOT 501. Only /agent-home short-circuits with 501.
|
||||
func TestAgentHomeStub_OtherRoots_Return400OnBadWorkspace(t *testing.T) {
|
||||
h, mock, _, _ := setupAgentHomeTest(t)
|
||||
|
||||
// Representative case: /configs with a bad workspace ID.
|
||||
// DB returns ErrNoRows → handler returns 404, not 501.
|
||||
mock.ExpectQuery(`SELECT name, COALESCE`).WithArgs("bad-ws").WillReturnError(sql.ErrNoRows)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root=/configs", nil)
|
||||
c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}}
|
||||
h.ListFiles(c)
|
||||
|
||||
// 404 → workspace not found (DB returned ErrNoRows), NOT 501.
|
||||
if w.Code == http.StatusNotImplemented {
|
||||
t.Error("/configs must NOT return 501 — only /agent-home short-circuits")
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
_ = h // suppress unused
|
||||
}
|
||||
@@ -19,12 +19,22 @@ import (
|
||||
|
||||
// allowedRoots are the container paths that the Files API can browse.
|
||||
var allowedRoots = map[string]bool{
|
||||
"/configs": true,
|
||||
"/workspace": true,
|
||||
"/home": true,
|
||||
"/plugins": true,
|
||||
"/configs": true,
|
||||
"/workspace": true,
|
||||
"/home": true,
|
||||
"/plugins": true,
|
||||
"/agent-home": true, // Phase 1 stub — docker-exec backend in Phase 2b (internal#425 RFC)
|
||||
}
|
||||
|
||||
// isAgentHomeStubRequest returns true when rootPath targets the /agent-home
|
||||
// root. Phase 1 returns 501; Phase 2b implements the docker-exec backend.
|
||||
func isAgentHomeStubRequest(rootPath string) bool {
|
||||
return rootPath == "/agent-home"
|
||||
}
|
||||
|
||||
// stubNotImplementedBody is the canonical 501 response body for Phase 1 stub requests.
|
||||
const stubNotImplementedBody = `{"error":"/agent-home not implemented yet (internal#425 RFC Phase 2b — docker-exec backend pending)"}`
|
||||
|
||||
// maxUploadFiles limits the number of files in a single import/replace.
|
||||
const maxUploadFiles = 200
|
||||
|
||||
@@ -219,7 +229,15 @@ func (h *TemplatesHandler) ListFiles(c *gin.Context) {
|
||||
// ?depth= — max depth to recurse (default: 1, max: 5)
|
||||
rootPath := c.DefaultQuery("root", "/configs")
|
||||
if !allowedRoots[rootPath] {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"})
|
||||
return
|
||||
}
|
||||
// Phase 1 stub — fires BEFORE the DB lookup so probe requests don't
|
||||
// generate "workspace not found" log noise. Phase 2b implements the
|
||||
// docker-exec backend (internal#425 RFC).
|
||||
if isAgentHomeStubRequest(rootPath) {
|
||||
c.Header("Content-Type", "application/json")
|
||||
c.String(http.StatusNotImplemented, stubNotImplementedBody)
|
||||
return
|
||||
}
|
||||
subPath := c.DefaultQuery("path", "")
|
||||
@@ -383,7 +401,13 @@ func (h *TemplatesHandler) ReadFile(c *gin.Context) {
|
||||
ctx := c.Request.Context()
|
||||
rootPath := c.DefaultQuery("root", "/configs")
|
||||
if !allowedRoots[rootPath] {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"})
|
||||
return
|
||||
}
|
||||
// Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b).
|
||||
if isAgentHomeStubRequest(rootPath) {
|
||||
c.Header("Content-Type", "application/json")
|
||||
c.String(http.StatusNotImplemented, stubNotImplementedBody)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -496,7 +520,13 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
|
||||
ctx := c.Request.Context()
|
||||
rootPath := c.DefaultQuery("root", "/configs")
|
||||
if !allowedRoots[rootPath] {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"})
|
||||
return
|
||||
}
|
||||
// Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b).
|
||||
if isAgentHomeStubRequest(rootPath) {
|
||||
c.Header("Content-Type", "application/json")
|
||||
c.String(http.StatusNotImplemented, stubNotImplementedBody)
|
||||
return
|
||||
}
|
||||
var wsName, instanceID, runtime string
|
||||
@@ -573,7 +603,13 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
|
||||
ctx := c.Request.Context()
|
||||
rootPath := c.DefaultQuery("root", "/configs")
|
||||
if !allowedRoots[rootPath] {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"})
|
||||
return
|
||||
}
|
||||
// Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b).
|
||||
if isAgentHomeStubRequest(rootPath) {
|
||||
c.Header("Content-Type", "application/json")
|
||||
c.String(http.StatusNotImplemented, stubNotImplementedBody)
|
||||
return
|
||||
}
|
||||
var wsName, instanceID, runtime string
|
||||
|
||||
Reference in New Issue
Block a user