fix(staging): wire OFFSEC-010 CP config + CWE-78 rows.Err fixes #1078
@ -116,6 +116,9 @@ func (h *ApprovalsHandler) ListAll(c *gin.Context) {
|
||||
"created_at": createdAt,
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ListPendingApprovals rows.Err: %v", err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, approvals)
|
||||
}
|
||||
@ -155,6 +158,9 @@ func (h *ApprovalsHandler) List(c *gin.Context) {
|
||||
"created_at": createdAt,
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ListApprovals rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, approvals)
|
||||
}
|
||||
|
||||
@ -248,6 +248,9 @@ func (h *InstructionsHandler) Resolve(c *gin.Context) {
|
||||
b.WriteString(content)
|
||||
b.WriteString("\n\n")
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ResolveInstructions rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
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 err := rows.Err(); err != nil {
|
||||
log.Printf("scanInstructions rows.Err: %v", err)
|
||||
}
|
||||
if instructions == nil {
|
||||
instructions = []Instruction{}
|
||||
}
|
||||
|
||||
@ -67,6 +67,9 @@ func (h *TokenHandler) List(c *gin.Context) {
|
||||
}
|
||||
tokens = append(tokens, t)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
log.Printf("ListTokens rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"tokens": tokens,
|
||||
|
||||
@ -805,6 +805,9 @@ func loadWorkspaceSecrets(ctx context.Context, workspaceID string) (map[string]s
|
||||
envVars[k] = string(decrypted)
|
||||
}
|
||||
}
|
||||
if err := globalRows.Err(); err != nil {
|
||||
log.Printf("Provisioner: global_secrets rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
}
|
||||
wsRows, err := db.DB.QueryContext(ctx,
|
||||
`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = $1`, workspaceID)
|
||||
@ -823,6 +826,9 @@ func loadWorkspaceSecrets(ctx context.Context, workspaceID string) (map[string]s
|
||||
envVars[k] = string(decrypted)
|
||||
}
|
||||
}
|
||||
if err := wsRows.Err(); err != nil {
|
||||
log.Printf("Provisioner: workspace_secrets rows.Err workspace=%s: %v", workspaceID, err)
|
||||
}
|
||||
}
|
||||
return envVars, ""
|
||||
}
|
||||
|
||||
@ -4,12 +4,14 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"log"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@ -156,6 +158,11 @@ type cpProvisionRequest struct {
|
||||
Tier int `json:"tier"`
|
||||
PlatformURL string `json:"platform_url"`
|
||||
Env map[string]string `json:"env"`
|
||||
// ConfigFiles are template + generated config files to write into the
|
||||
// EC2 instance's /configs directory. OFFSEC-010: collected by
|
||||
// collectCPConfigFiles which rejects symlinks and non-regular files
|
||||
// before including them. Serialised as base64 to avoid JSON escaping.
|
||||
ConfigFiles map[string]string `json:"config_files,omitempty"`
|
||||
}
|
||||
|
||||
type cpProvisionResponse struct {
|
||||
@ -179,6 +186,16 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string,
|
||||
}
|
||||
env["ADMIN_TOKEN"] = p.adminToken
|
||||
}
|
||||
// Collect template files and generated configs, with OFFSEC-010 guards:
|
||||
// - Rejects symlinks at the template root (prevents bypass via symlink traversal)
|
||||
// - Skips symlinks during WalkDir (prevents /etc/passwd etc. inclusion)
|
||||
// - Validates all paths are relative and non-escaping
|
||||
// - Caps total size at 12 KiB to prevent payload bloat
|
||||
configFiles, err := collectCPConfigFiles(cfg)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("cp provisioner: collect config files: %w", err)
|
||||
}
|
||||
|
||||
req := cpProvisionRequest{
|
||||
OrgID: p.orgID,
|
||||
WorkspaceID: cfg.WorkspaceID,
|
||||
@ -186,6 +203,7 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string,
|
||||
Tier: cfg.Tier,
|
||||
PlatformURL: cfg.PlatformURL,
|
||||
Env: env,
|
||||
ConfigFiles: configFiles,
|
||||
}
|
||||
|
||||
body, err := json.Marshal(req)
|
||||
@ -237,6 +255,94 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string,
|
||||
return result.InstanceID, nil
|
||||
}
|
||||
|
||||
const cpConfigFilesMaxBytes = 12 << 10
|
||||
|
||||
// isCPTemplateConfigFile restricts which files from a template directory are
|
||||
// eligible for transport to the control plane. Only config.yaml (the runtime
|
||||
// entrypoint config) and files under prompts/ (system prompts) are needed;
|
||||
// shipping arbitrary files (e.g. adapter.py, Dockerfile) is both unnecessary
|
||||
// and a potential data-exfiltration surface.
|
||||
func isCPTemplateConfigFile(name string) bool {
|
||||
name = filepath.ToSlash(filepath.Clean(name))
|
||||
return name == "config.yaml" || strings.HasPrefix(name, "prompts/")
|
||||
}
|
||||
|
||||
func collectCPConfigFiles(cfg WorkspaceConfig) (map[string]string, error) {
|
||||
files := make(map[string]string)
|
||||
total := 0
|
||||
addFile := func(name string, data []byte) error {
|
||||
name = filepath.ToSlash(filepath.Clean(name))
|
||||
if name == "." || strings.HasPrefix(name, "../") || strings.HasPrefix(name, "/") || strings.Contains(name, "/../") {
|
||||
return fmt.Errorf("invalid config file path %q", name)
|
||||
}
|
||||
total += len(data)
|
||||
if total > cpConfigFilesMaxBytes {
|
||||
return fmt.Errorf("config files exceed %d bytes", cpConfigFilesMaxBytes)
|
||||
}
|
||||
files[name] = base64.StdEncoding.EncodeToString(data)
|
||||
return nil
|
||||
}
|
||||
|
||||
if cfg.TemplatePath != "" {
|
||||
// Reject symlinks on the root itself — WalkDir follows symlinks,
|
||||
// so a symlink TemplatePath that escapes the intended root directory
|
||||
// would bypass the subsequent path-relativization checks below.
|
||||
rootInfo, err := os.Lstat(cfg.TemplatePath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("collectCPConfigFiles: lstat template path: %w", err)
|
||||
}
|
||||
if rootInfo.Mode()&os.ModeSymlink != 0 {
|
||||
return nil, fmt.Errorf("collectCPConfigFiles: template path must not be a symlink")
|
||||
}
|
||||
err = filepath.WalkDir(cfg.TemplatePath, func(path string, d os.DirEntry, walkErr error) error {
|
||||
if walkErr != nil {
|
||||
return walkErr
|
||||
}
|
||||
// Skip symlinks — WalkDir follows them by default, which means
|
||||
// a symlink inside the template dir pointing to /etc/passwd
|
||||
// would be traversed even though the resulting relative-path
|
||||
// check would correctly reject it. Defense-in-depth: don't
|
||||
// follow symlinks at all. (OFFSEC-010)
|
||||
if d.Type()&os.ModeSymlink != 0 {
|
||||
return nil
|
||||
}
|
||||
if d.IsDir() {
|
||||
return nil
|
||||
}
|
||||
info, err := d.Info()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
return nil
|
||||
}
|
||||
rel, err := filepath.Rel(cfg.TemplatePath, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !isCPTemplateConfigFile(rel) {
|
||||
return nil
|
||||
}
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return addFile(rel, data)
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
for name, data := range cfg.ConfigFiles {
|
||||
if err := addFile(name, data); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
if len(files) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
return files, nil
|
||||
}
|
||||
// Stop terminates the workspace's EC2 instance via the control plane.
|
||||
//
|
||||
// Looks up the actual EC2 instance_id from the workspaces table before
|
||||
|
||||
@ -1,11 +1,15 @@
|
||||
package provisioner
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@ -279,6 +283,105 @@ func TestStart_TransportFailureSurfaces(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestStart_CollectsConfigFiles — verify that collectCPConfigFiles is called and
|
||||
// its result is included in the cpProvisionRequest sent to the control plane.
|
||||
// Tests the OFFSEC-010 wiring: the function's symlink guards are only effective
|
||||
// if the call site actually invokes it.
|
||||
func TestStart_CollectsConfigFiles(t *testing.T) {
|
||||
tmpl := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(tmpl, "config.yaml"), []byte("name: test\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// adapter.py is within the size limit but is NOT config.yaml or prompts/,
|
||||
// so isCPTemplateConfigFile must exclude it from the transport.
|
||||
if err := os.WriteFile(filepath.Join(tmpl, "adapter.py"), bytes.Repeat([]byte("x"), cpConfigFilesMaxBytes), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var gotBody cpProvisionRequest
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = json.NewDecoder(r.Body).Decode(&gotBody)
|
||||
w.WriteHeader(http.StatusCreated)
|
||||
_, _ = io.WriteString(w, `{"instance_id":"i-abc123","state":"pending"}`)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()}
|
||||
_, err := p.Start(context.Background(), WorkspaceConfig{
|
||||
WorkspaceID: "ws-1",
|
||||
Runtime: "python",
|
||||
Tier: 1,
|
||||
PlatformURL: "http://tenant",
|
||||
TemplatePath: tmpl,
|
||||
ConfigFiles: map[string][]byte{"generated.json": []byte(`{"key":"value"}`)},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Start: %v", err)
|
||||
}
|
||||
|
||||
// config.yaml from TemplatePath must be base64-encoded in ConfigFiles
|
||||
if len(gotBody.ConfigFiles) == 0 {
|
||||
t.Fatal("ConfigFiles is empty: collectCPConfigFiles was not called")
|
||||
}
|
||||
|
||||
// Find config.yaml entry and verify it's valid base64 + correct content
|
||||
var foundTemplate, foundGenerated bool
|
||||
for name, encoded := range gotBody.ConfigFiles {
|
||||
decoded, err := base64.StdEncoding.DecodeString(encoded)
|
||||
if err != nil {
|
||||
t.Errorf("ConfigFiles[%q] is not valid base64: %v", name, err)
|
||||
continue
|
||||
}
|
||||
if name == "config.yaml" && string(decoded) == "name: test\n" {
|
||||
foundTemplate = true
|
||||
}
|
||||
if name == "generated.json" && string(decoded) == `{"key":"value"}` {
|
||||
foundGenerated = true
|
||||
}
|
||||
}
|
||||
if !foundTemplate {
|
||||
t.Errorf("ConfigFiles missing config.yaml from TemplatePath")
|
||||
}
|
||||
if !foundGenerated {
|
||||
t.Errorf("ConfigFiles missing generated.json from ConfigFiles")
|
||||
}
|
||||
// adapter.py must NOT be in ConfigFiles — isCPTemplateConfigFile filters it out
|
||||
for name := range gotBody.ConfigFiles {
|
||||
if name == "adapter.py" {
|
||||
t.Errorf("adapter.py should not be in ConfigFiles — isCPTemplateConfigFile must filter it out")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestStart_SymlinkTemplatePathError — a symlink TemplatePath should cause
|
||||
// collectCPConfigFiles to return an error, which Start must propagate.
|
||||
// Without this wiring, OFFSEC-010's root-symlink guard is dead code.
|
||||
func TestStart_SymlinkTemplatePathError(t *testing.T) {
|
||||
// Create a temp file and a symlink pointing to it
|
||||
tmp := t.TempDir()
|
||||
realFile := filepath.Join(tmp, "real")
|
||||
if err := os.WriteFile(realFile, []byte("data"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
symlink := filepath.Join(tmp, "template_link")
|
||||
if err := os.Symlink(realFile, symlink); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
p := &CPProvisioner{baseURL: "http://unused", orgID: "org-1", httpClient: &http.Client{Timeout: time.Second}}
|
||||
_, err := p.Start(context.Background(), WorkspaceConfig{
|
||||
WorkspaceID: "ws-1",
|
||||
Runtime: "python",
|
||||
TemplatePath: symlink, // symlink root → OFFSEC-010 guard should fire
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected error for symlink TemplatePath, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "symlink") {
|
||||
t.Errorf("error should mention symlink, got %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// TestStop_SendsBothAuthHeaders — verify #118/#130 compliance on the
|
||||
// teardown path. Any call to /cp/workspaces/:id must carry both the
|
||||
// platform-wide shared secret AND the per-tenant admin token, or the
|
||||
@ -842,3 +945,67 @@ func TestIsRunning_EmptyInstanceIDReturnsFalse(t *testing.T) {
|
||||
t.Errorf("IsRunning with empty instance_id should return running=false, got true")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCollectCPConfigFiles_SkipsSymlinks — WalkDir follows symlinks by default,
|
||||
// but collectCPConfigFiles must skip them so a symlink inside a template dir
|
||||
// pointing outside (e.g. ln -s /etc snapshot) cannot be traversed.
|
||||
// Verifies OFFSEC-010 defense-in-depth fix. (OFFSEC-010)
|
||||
func TestCollectCPConfigFiles_SkipsSymlinks(t *testing.T) {
|
||||
tmpl := t.TempDir()
|
||||
// Write a real file that should be included.
|
||||
if err := os.WriteFile(filepath.Join(tmpl, "config.yaml"), []byte("name: real\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Create a subdir with a file that will be symlinked-outside.
|
||||
sensitiveDir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(sensitiveDir, "secret.txt"), []byte("SENSITIVE\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Symlink inside template dir pointing to outside path.
|
||||
symlinkPath := filepath.Join(tmpl, "snapshot")
|
||||
if err := os.Symlink(sensitiveDir, symlinkPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
files, err := collectCPConfigFiles(WorkspaceConfig{TemplatePath: tmpl})
|
||||
if err != nil {
|
||||
t.Fatalf("collectCPConfigFiles: %v", err)
|
||||
}
|
||||
if files == nil {
|
||||
t.Fatal("files should not be nil")
|
||||
}
|
||||
// config.yaml must be present.
|
||||
if _, ok := files["config.yaml"]; !ok {
|
||||
t.Errorf("config.yaml missing from files")
|
||||
}
|
||||
// The symlinked path must NOT be included (even though WalkDir would
|
||||
// traverse it, the d.Type()&os.ModeSymlink guard skips the entry).
|
||||
for k := range files {
|
||||
if strings.Contains(k, "snapshot") || strings.Contains(k, "secret") {
|
||||
t.Errorf("symlink path %q should not be in files — OFFSEC-010 regression", k)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCollectCPConfigFiles_RejectsRootSymlink — if cfg.TemplatePath itself is
|
||||
// a symlink, WalkDir would follow it to an arbitrary directory, bypassing the
|
||||
// cfg.TemplatePath boundary. The function must reject this case explicitly.
|
||||
// (OFFSEC-010)
|
||||
func TestCollectCPConfigFiles_RejectsRootSymlink(t *testing.T) {
|
||||
real := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(real, "config.yaml"), []byte("name: real\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
link := filepath.Join(t.TempDir(), "template-link")
|
||||
if err := os.Symlink(real, link); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, err := collectCPConfigFiles(WorkspaceConfig{TemplatePath: link})
|
||||
if err == nil {
|
||||
t.Error("collectCPConfigFiles with symlink TemplatePath should return error")
|
||||
}
|
||||
if err != nil && !strings.Contains(err.Error(), "symlink") {
|
||||
t.Errorf("expected symlink-related error, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user