mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(auth): harden Windows storage bridge

Browse Source
This commit is contained in:
marcopan
2026-08-16 22:16:13 +02:00
parent c9b02fc57e
commit 7d9ca13f1d
12 changed files with 473 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/tht/internal/authstorage/storage.go
+7 -2
View File
@@ -3,6 +3,7 @@
package authstorage
import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
@@ -60,7 +61,11 @@ func Run(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io.
if err := ctx.Err(); err != nil {
return fail(stderr)
}
decoder := json.NewDecoder(io.LimitReader(stdin, maximumProtocolBytes+1))
payload, err := io.ReadAll(io.LimitReader(stdin, maximumProtocolBytes+1))
if err != nil || len(payload) > maximumProtocolBytes {
return fail(stderr)
}
decoder := json.NewDecoder(bytes.NewReader(payload))
decoder.DisallowUnknownFields()
var input request
if err := decoder.Decode(&input); err != nil {
@@ -229,7 +234,7 @@ func createPrivate(directory, filename string, contents []byte) (bool, error) {
} else if !errors.Is(err, os.ErrNotExist) {
return false, errInvalid
}
if err := safeio.WriteCanonicalNewFile(path, contents, 0o600); err == nil {
if err := safeio.WriteCanonicalNewPrivateFile(path, contents, 0o600); err == nil {
return true, nil
}
if safeio.ValidatePrivateRegular(path) == nil {
tools/tht/internal/authstorage/storage_test.go
+47
View File
@@ -7,6 +7,7 @@ import (
"encoding/json"
"os"
"path/filepath"
"strings"
"sync"
"testing"
@@ -43,6 +44,44 @@ func TestProtocolCreatesReadsReplacesListsAndRemovesPrivateRecord(t *testing.T)
}
}
func TestProtocolListSerializesLowerCamelBridgeDTO(t *testing.T) {
root := filepath.Join(privateTestRoot(t), "auth")
filename := "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc.json"
runRequest(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))})
input, err := json.Marshal(request{Version: 1, Operation: "list", Root: root, Directory: "sessions"})
if err != nil {
t.Fatal(err)
}
var stdout, stderr bytes.Buffer
if code := Run(context.Background(), nil, bytes.NewReader(input), &stdout, &stderr); code != 0 {
t.Fatalf("Run() code = %d stderr = %q", code, stderr.String())
}
if !strings.Contains(stdout.String(), `"name":"`+filename+`"`) || strings.Contains(stdout.String(), `"Name":`) {
t.Fatalf("list bridge JSON = %q, want lower-camel entry fields", stdout.String())
}
}
func TestRunRejectsProtocolOverflowAndTrailingJSONValues(t *testing.T) {
root := filepath.Join(privateTestRoot(t), "auth")
valid, err := json.Marshal(request{Version: 1, Operation: "list", Root: root, Directory: "sessions"})
if err != nil {
t.Fatal(err)
}
if len(valid) >= maximumProtocolBytes {
t.Fatal("test request unexpectedly consumes protocol bound")
}
overflowWhitespace := append(append([]byte(nil), valid...), bytes.Repeat([]byte(" "), maximumProtocolBytes+1-len(valid))...)
runRawRejected(t, overflowWhitespace)
withinBound := append(append([]byte(nil), valid...), []byte("{}")...)
runRawRejected(t, withinBound)
crossingBound := append(append([]byte(nil), valid...), bytes.Repeat([]byte(" "), maximumProtocolBytes-len(valid)-1)...)
crossingBound = append(crossingBound, []byte("{}")...)
runRawRejected(t, crossingBound)
}
func TestProtocolClaimConsumeIsAtomicAcrossConcurrentRequests(t *testing.T) {
root := filepath.Join(privateTestRoot(t), "auth")
filename := "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd.json"
@@ -140,6 +179,14 @@ func runRejected(t *testing.T, value request) {
}
}
func runRawRejected(t *testing.T, input []byte) {
t.Helper()
var stdout, stderr bytes.Buffer
if code := Run(context.Background(), nil, bytes.NewReader(input), &stdout, &stderr); code == 0 || stdout.Len() != 0 || stderr.String() != "tht: auth storage request failed\n" {
t.Fatalf("Run() rejection code=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String())
}
}
func decodeContent(t *testing.T, value response) string {
t.Helper()
contents, err := base64.StdEncoding.DecodeString(value.ContentBase64)
tools/tht/internal/authstorage/storage_windows_test.go
+10
View File
@@ -45,6 +45,16 @@ func TestProtocolRejectsPermissiveDACLAndReparseRoot(t *testing.T) {
runRejected(t, request{Version: 1, Operation: "create", Root: linkedRoot, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("private"))})
}
func TestProtocolRejectsRecordCreationAfterSessionsDirectoryDACLBecomesPermissive(t *testing.T) {
root := filepath.Join(t.TempDir(), "auth")
filename := "abababababababababababababababababababababababababababababababab.json"
runRequest(t, request{Version: 1, Operation: "list", Root: root, Directory: "sessions"})
if err := setPermissiveDACL(filepath.Join(root, "sessions")); err != nil {
t.Fatal(err)
}
runRejected(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))})
}
func setPermissiveDACL(path string) error {
world, err := windows.StringToSid("S-1-1-0")
if err != nil {
tools/tht/internal/safeio/files.go
+37 -5
View File
@@ -8,6 +8,7 @@ import (
"io"
"os"
"path/filepath"
"sort"
"strings"
"unicode/utf8"
)
@@ -81,8 +82,8 @@ func ReadCanonicalPrivateRegular(path string, maximum int64) ([]byte, error) {
// PrivateDirectoryEntry is a bounded, untrusted directory listing item. Callers must still
// validate each filename and record before using it.
type PrivateDirectoryEntry struct {
Name string
ModifiedUnixMs int64
Name string `json:"name"`
ModifiedUnixMs int64 `json:"modifiedUnixMs"`
}
// ListCanonicalPrivateDirectory lists regular, non-symlinked direct children from an owner-only
@@ -94,8 +95,16 @@ func ListCanonicalPrivateDirectory(path string, maximumEntries int) ([]PrivateDi
if err := ValidatePrivateDirectory(path); err != nil {
return nil, ErrUnsafeFile
}
entries, err := os.ReadDir(path)
if err != nil || len(entries) > maximumEntries {
directory, err := os.Open(path)
if err != nil {
return nil, ErrUnsafeFile
}
defer directory.Close()
entries, err := directory.ReadDir(maximumEntries + 1)
if err != nil && !errors.Is(err, io.EOF) {
return nil, ErrUnsafeFile
}
if len(entries) > maximumEntries {
return nil, ErrUnsafeFile
}
result := make([]PrivateDirectoryEntry, 0, len(entries))
@@ -109,6 +118,7 @@ func ListCanonicalPrivateDirectory(path string, maximumEntries int) ([]PrivateDi
}
result = append(result, PrivateDirectoryEntry{Name: entry.Name(), ModifiedUnixMs: info.ModTime().UnixMilli()})
}
sort.Slice(result, func(left, right int) bool { return result[left].Name < result[right].Name })
if err := ValidatePrivateDirectory(path); err != nil {
return nil, ErrUnsafeFile
}
@@ -116,6 +126,17 @@ func ListCanonicalPrivateDirectory(path string, maximumEntries int) ([]PrivateDi
}
func WriteCanonicalNewFile(path string, contents []byte, mode os.FileMode) error {
return writeCanonicalNewFile(path, contents, mode, false)
}
// WriteCanonicalNewPrivateFile is the authentication-storage variant of exclusive file
// creation. It requires the final parent directory to already have platform-specific owner-only
// protection and preserves that check while the platform primitive opens the parent.
func WriteCanonicalNewPrivateFile(path string, contents []byte, mode os.FileMode) error {
return writeCanonicalNewFile(path, contents, mode, true)
}
func writeCanonicalNewFile(path string, contents []byte, mode os.FileMode, requirePrivateParent bool) error {
if err := ValidateCanonicalPath(path); err != nil {
return err
}
@@ -123,6 +144,9 @@ func WriteCanonicalNewFile(path string, contents []byte, mode os.FileMode) error
if err := requireCanonicalDirectory(parent); err != nil {
return err
}
if requirePrivateParent && ValidatePrivateDirectory(parent) != nil {
return ErrUnsafeFile
}
if info, err := os.Lstat(path); err == nil {
if !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 || info.Mode()&os.ModeType != 0 {
return ErrUnsafeFile
@@ -131,7 +155,15 @@ func WriteCanonicalNewFile(path string, contents []byte, mode os.FileMode) error
} else if !errors.Is(err, os.ErrNotExist) {
return ErrUnsafeFile
}
file, err := createCanonicalNewPrivateFile(path, mode)
var (
file *os.File
err error
)
if requirePrivateParent {
file, err = createCanonicalNewPrivateParentFile(path, mode)
} else {
file, err = createCanonicalNewPrivateFile(path, mode)
}
if err != nil {
return ErrUnsafeFile
}
tools/tht/internal/safeio/files_test.go
+32
View File
@@ -2,6 +2,7 @@ package safeio
import (
"errors"
"fmt"
"os"
"path/filepath"
"testing"
@@ -93,3 +94,34 @@ func TestWriteCanonicalNewFileRejectsExistingTargets(t *testing.T) {
t.Fatalf("WriteCanonicalNewFile(existing) error = %v, want ErrUnsafeFile", err)
}
}
func TestListCanonicalPrivateDirectoryBoundsAndSortsValidatedEntries(t *testing.T) {
temporaryRoot, err := filepath.EvalSymlinks(os.TempDir())
if err != nil {
t.Fatal(err)
}
root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-list-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(root) })
if err := ProtectPrivateDirectory(root); err != nil {
t.Fatal(err)
}
for index := 255; index >= 0; index-- {
path := filepath.Join(root, fmt.Sprintf("%064x.json", index))
if err := WriteCanonicalNewFile(path, []byte("record"), 0o600); err != nil {
t.Fatalf("WriteCanonicalNewFile(%d) error = %v", index, err)
}
}
entries, err := ListCanonicalPrivateDirectory(root, 256)
if err != nil || len(entries) != 256 || entries[0].Name != fmt.Sprintf("%064x.json", 0) || entries[255].Name != fmt.Sprintf("%064x.json", 255) {
t.Fatalf("bounded ordered entries = %#v error = %v", entries, err)
}
if err := WriteCanonicalNewFile(filepath.Join(root, fmt.Sprintf("%064x.json", 256)), []byte("record"), 0o600); err != nil {
t.Fatal(err)
}
if _, err := ListCanonicalPrivateDirectory(root, 256); !errors.Is(err, ErrUnsafeFile) {
t.Fatalf("257-entry listing error = %v, want ErrUnsafeFile", err)
}
}
tools/tht/internal/safeio/private_unix.go
+7
View File
@@ -65,6 +65,13 @@ func createCanonicalNewPrivateFile(path string, mode os.FileMode) (*os.File, err
return file, nil
}
func createCanonicalNewPrivateParentFile(path string, mode os.FileMode) (*os.File, error) {
if err := ValidatePrivateDirectory(filepath.Dir(path)); err != nil {
return nil, ErrUnsafeFile
}
return createCanonicalNewPrivateFile(path, mode)
}
// ValidatePrivateRegular requires a canonical, single-link private regular file.
func ValidatePrivateRegular(path string) error {
if err := ValidateCanonicalPath(path); err != nil {
tools/tht/internal/safeio/private_windows.go
+9 -1
View File
@@ -96,8 +96,16 @@ func ProtectPrivateRegular(path string) error {
// createCanonicalNewPrivateFile installs the owner-only protected DACL in the CreateFile call, so
// another mutation can never observe a newly-created lock with an inherited/default DACL.
func createCanonicalNewPrivateFile(path string, mode os.FileMode) (*os.File, error) {
return createCanonicalNewFile(path, mode, false)
}
func createCanonicalNewPrivateParentFile(path string, mode os.FileMode) (*os.File, error) {
return createCanonicalNewFile(path, mode, true)
}
func createCanonicalNewFile(path string, mode os.FileMode, requirePrivateParent bool) (*os.File, error) {
parents, target, err := openCanonicalWindowsParent(path)
if err != nil || len(parents.handles) == 0 || validateOwnerOnlyDACL(parents.handles[len(parents.handles)-1]) != nil {
if err != nil || len(parents.handles) == 0 || (requirePrivateParent && validateOwnerOnlyDACL(parents.handles[len(parents.handles)-1]) != nil) {
if parents != nil {
parents.Close()
}
tools/tht/internal/safeio/private_windows_test.go
+16
View File
@@ -73,6 +73,22 @@ func TestCreateCanonicalNewPrivateFileInstallsOwnerOnlyDACLAtCreation(t *testing
}
}
func TestGenericNewFileAllowsInheritedOperatorParentButAuthNewFileRequiresPrivateParent(t *testing.T) {
directory := filepath.Join(t.TempDir(), "operator-output")
if err := os.Mkdir(directory, 0o700); err != nil {
t.Fatal(err)
}
if err := setPermissiveDACL(directory); err != nil {
t.Fatal(err)
}
if err := WriteCanonicalNewFile(filepath.Join(directory, "candidates.yaml"), []byte("reviewed: []\n"), 0o600); err != nil {
t.Fatalf("generic operator output error = %v", err)
}
if err := WriteCanonicalNewPrivateFile(filepath.Join(directory, "auth.json"), []byte("record"), 0o600); !errors.Is(err, ErrUnsafeFile) {
t.Fatalf("auth record in inherited directory error = %v, want ErrUnsafeFile", err)
}
}
func TestWithWindowsSecurityDescriptorKeepsOwnedDescriptorValidDuringInspection(t *testing.T) {
directory := filepath.Join(t.TempDir(), "auth")
if err := os.Mkdir(directory, 0o700); err != nil {
tools/tht/internal/workspaceops/operations_windows_test.go
+44
View File
@@ -0,0 +1,44 @@
//go:build windows
package workspaceops
import (
"context"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"github.com/aritmolab/thothii/tools/tht/internal/compose"
)
func TestExecuteSuggestFksWritesOutputInInheritedOperatorDirectory(t *testing.T) {
installation := testInstallation(t)
operatorDirectory := filepath.Join(filepath.Dir(installation.Path), "operator-output")
if err := os.Mkdir(operatorDirectory, 0o700); err != nil {
t.Fatal(err)
}
output := filepath.Join(operatorDirectory, "candidates.yaml")
runner := &fakeRunner{run: func(args []string, _ string) (compose.Result, error) {
switch {
case contains(args, "config", "--format", "json"):
return compose.Result{Stdout: `{"services":{"core":{"image":"thothii-core:local"}}}`}, nil
case reflect.DeepEqual(args, []string{"image", "inspect", "--format", "{{.Id}}", "thothii-core:local"}):
return compose.Result{Stdout: "sha256:" + strings.Repeat("d", 64)}, nil
case contains(args, "workspace-maintenance", "schema-suggest-fks"):
return compose.Result{Stdout: `{"schemaVersion":1,"status":"blocked","code":"manual_review_required","workspaceId":"abc","workspaceRevision":"1234567890abcdef1234567890abcdef12345678","descriptorBlob":"sha256:` + strings.Repeat("e", 64) + `","operation":"schema-suggest-fks","completedStages":[],"suggestedFksYaml":"reviewed: []\n"}`}, nil
default:
t.Fatalf("unexpected Docker invocation: %#v", args)
return compose.Result{}, nil
}
}}
if _, err := Execute(context.Background(), installation, runner, SuggestFksRequest{baseRequest: baseRequest{Workspace: "abc"}, Output: output}); err != nil {
t.Fatalf("workspace schema suggest-fks --output error = %v", err)
}
contents, err := os.ReadFile(output)
if err != nil || string(contents) != "reviewed: []\n" {
t.Fatalf("workspace schema suggest-fks output = %q error = %v", contents, err)
}
}