fix(provisioner): skip symlinks in collectCPConfigFiles WalkDir (OFFSEC-010) #1075

Merged
devops-engineer merged 1 commits from fix/offsec-010-clean into main 2026-05-14 21:36:38 +00:00
2 changed files with 141 additions and 0 deletions

View File

@ -4,12 +4,14 @@ import (
"bytes"
"context"
"database/sql"
"encoding/base64"
"encoding/json"
"fmt"
"io"
"log"
"net/http"
"os"
"path/filepath"
"strings"
"time"
@ -237,6 +239,81 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string,
return result.InstanceID, nil
}
const cpConfigFilesMaxBytes = 12 << 10
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
}
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

View File

@ -842,3 +842,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)
}
}