Skip to content

Commit 431ea27

Browse files
authored
Kody reveiw fixes (#341)
* Kody reveiw fixes * Update fix * More flexible gateway enclave stuff for older macbooks * Further hardening
1 parent bdbc305 commit 431ea27

File tree

11 files changed

+423
-57
lines changed

11 files changed

+423
-57
lines changed

internal/inference/gateway.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -188,19 +188,19 @@ func (g *Gateway) buildHandler(upstreamURL string) (http.Handler, error) {
188188
g.config.TEEType, seKey.Tag(), seKey.PublicKeyBytes()[:8])
189189

190190
case g.config.EnclaveTag != "":
191-
// macOS Secure Enclave path (existing).
191+
// macOS Secure Enclave path. If the hardware isn't available (older
192+
// Intel Macs without T2, or SIP is disabled), fall back to running
193+
// without transit encryption rather than failing the entire gateway.
194+
// The x402 payment gate still protects the endpoint.
192195
if err := enclave.CheckSIP(); err != nil {
193-
return nil, fmt.Errorf("enclave SIP check failed: %w", err)
196+
log.Printf(" enclave: unavailable (SIP check: %v) — continuing without transit encryption", err)
197+
} else if em, err = newEnclaveMiddleware(g.config.EnclaveTag); err != nil {
198+
log.Printf(" enclave: unavailable (%v) — continuing without transit encryption", err)
199+
} else {
200+
g.seKey = em.key
201+
log.Printf(" enclave: tag=%q persistent=%v pubkey=%x...",
202+
em.key.Tag(), em.key.Persistent(), em.key.PublicKeyBytes()[:8])
194203
}
195-
196-
em, err = newEnclaveMiddleware(g.config.EnclaveTag)
197-
if err != nil {
198-
return nil, fmt.Errorf("enclave middleware: %w", err)
199-
}
200-
201-
g.seKey = em.key
202-
log.Printf(" enclave: tag=%q persistent=%v pubkey=%x...",
203-
em.key.Tag(), em.key.Persistent(), em.key.PublicKeyBytes()[:8])
204204
}
205205

206206
// protect wraps a handler with the payment gate and (when enabled) the SE

internal/inference/store.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"path/filepath"
99
"regexp"
10+
"runtime"
1011
"time"
1112

1213
x402verifier "github.com/ObolNetwork/obol-stack/internal/x402"
@@ -162,7 +163,11 @@ func (s *Store) Create(d *Deployment, force bool) error {
162163
}
163164

164165
// Apply defaults.
165-
if d.EnclaveTag == "" {
166+
// Auto-assign an enclave tag only on macOS where the Secure Enclave is
167+
// available. On Linux, the gateway runs without transit encryption unless
168+
// an explicit --tee flag is provided. The payment gate still protects the
169+
// endpoint; encryption is an additional layer, not a requirement.
170+
if d.EnclaveTag == "" && runtime.GOOS == "darwin" {
166171
d.EnclaveTag = "com.obol.inference." + d.Name
167172
}
168173

internal/inference/store_test.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package inference_test
33
import (
44
"errors"
55
"os"
6+
"runtime"
67
"testing"
78

89
"github.com/ObolNetwork/obol-stack/internal/inference"
@@ -21,9 +22,17 @@ func TestStoreCreateAndGet(t *testing.T) {
2122
t.Fatalf("Create: %v", err)
2223
}
2324

24-
// Defaults should be applied.
25-
if d.EnclaveTag == "" {
26-
t.Error("EnclaveTag should have been set by Create")
25+
// EnclaveTag is auto-assigned only on macOS (Secure Enclave).
26+
// On Linux the field stays empty so the gateway runs without the
27+
// enclave middleware (payment gating still works).
28+
if runtime.GOOS == "darwin" {
29+
if d.EnclaveTag == "" {
30+
t.Error("EnclaveTag should have been set by Create on macOS")
31+
}
32+
} else {
33+
if d.EnclaveTag != "" {
34+
t.Errorf("EnclaveTag should be empty on %s, got %q", runtime.GOOS, d.EnclaveTag)
35+
}
2736
}
2837

2938
if d.Chain == "" {
@@ -50,8 +59,14 @@ func TestStoreCreateAndGet(t *testing.T) {
5059
t.Errorf("WalletAddress mismatch: %s", got.WalletAddress)
5160
}
5261

53-
if got.EnclaveTag != "com.obol.inference.test-deploy" {
54-
t.Errorf("unexpected EnclaveTag: %s", got.EnclaveTag)
62+
if runtime.GOOS == "darwin" {
63+
if got.EnclaveTag != "com.obol.inference.test-deploy" {
64+
t.Errorf("unexpected EnclaveTag: %s", got.EnclaveTag)
65+
}
66+
} else {
67+
if got.EnclaveTag != "" {
68+
t.Errorf("EnclaveTag should be empty on %s, got %q", runtime.GOOS, got.EnclaveTag)
69+
}
5570
}
5671
if got.Chain != "base" {
5772
t.Errorf("persisted Chain = %q, want %q", got.Chain, "base")

internal/openclaw/openclaw.go

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ func copyWorkspaceToVolume(cfg *config.Config, id, workspaceDir string, u *ui.UI
615615
u.Blank()
616616
u.Infof("Importing workspace from %s...", workspaceDir)
617617

618+
ensureVolumeWritable(cfg, targetDir, u)
618619
if err := os.MkdirAll(targetDir, 0o755); err != nil {
619620
u.Warnf("could not create workspace directory: %v", err)
620621
return
@@ -706,6 +707,7 @@ func injectSkillsToVolume(cfg *config.Config, id string, deploymentDir string, u
706707
}
707708

708709
targetDir := skillsVolumePath(cfg, id)
710+
ensureVolumeWritable(cfg, targetDir, u)
709711
if err := os.MkdirAll(targetDir, 0o755); err != nil {
710712
u.Warnf("could not create skills volume directory: %v", err)
711713
return
@@ -732,54 +734,93 @@ func injectSkillsToVolume(cfg *config.Config, id string, deploymentDir string, u
732734
fixVolumeOwnership(cfg, targetDir, u)
733735
}
734736

737+
// k3dNodeExec runs a shell command inside the k3d node container, translating
738+
// the host-side path to the in-node path (/data/…). Returns an error when
739+
// the command fails or the path is outside the data directory. This is the
740+
// shared core for fixVolumeOwnership and ensureVolumeWritable.
741+
func k3dNodeExec(cfg *config.Config, hostPath, shellCmd string) error {
742+
stackID := ""
743+
if data, err := os.ReadFile(filepath.Join(cfg.ConfigDir, ".stack-id")); err == nil {
744+
stackID = strings.TrimSpace(string(data))
745+
}
746+
if stackID == "" {
747+
return fmt.Errorf("stack ID not found")
748+
}
749+
750+
container := fmt.Sprintf("k3d-obol-stack-%s-server-0", stackID)
751+
752+
// Convert host path to the in-node path. k3d mounts $DATA_DIR → /data.
753+
relPath, err := filepath.Rel(cfg.DataDir, hostPath)
754+
if err != nil {
755+
return fmt.Errorf("cannot compute relative path from %s to %s: %w", cfg.DataDir, hostPath, err)
756+
}
757+
if strings.HasPrefix(relPath, "..") {
758+
return fmt.Errorf("path %s is not under DataDir %s", hostPath, cfg.DataDir)
759+
}
760+
nodePath := filepath.Join("/data", relPath)
761+
762+
// Replace the placeholder with the shell-quoted node path.
763+
quoted := "'" + strings.ReplaceAll(nodePath, "'", "'\"'\"'") + "'"
764+
expanded := strings.ReplaceAll(shellCmd, "{}", quoted)
765+
766+
cmd := exec.Command("docker", "exec", container, "sh", "-c", expanded)
767+
return cmd.Run()
768+
}
769+
735770
// fixVolumeOwnership normalises file ownership on a host-side PVC path so the
736-
// container (UID 1000 / node) can read and write. On k3d the host path is
771+
// container (UID 1000 / node) can read and write. On k3d the host path is
737772
// inside a Docker container (the k3d node), so we exec into it as root and
738-
// chown recursively. On k3s the host IS the node, so we attempt a direct
773+
// chown recursively. On k3s the host IS the node, so we attempt a direct
739774
// chown (works when the CLI runs as root, harmless no-op otherwise).
740775
//
741776
// The optional ui parameter enables user-visible warnings when chown fails.
742777
// Pass nil when no UI context is available (e.g. GenerateWallet).
743778
func fixVolumeOwnership(cfg *config.Config, hostPath string, u *ui.UI) {
744-
// Determine backend (default: k3d for backward compat).
745779
backendName := "k3d"
746780
if data, err := os.ReadFile(filepath.Join(cfg.ConfigDir, ".stack-backend")); err == nil {
747781
backendName = strings.TrimSpace(string(data))
748782
}
749783

750784
switch backendName {
751785
case "k3d":
752-
stackID := ""
753-
if data, err := os.ReadFile(filepath.Join(cfg.ConfigDir, ".stack-id")); err == nil {
754-
stackID = strings.TrimSpace(string(data))
755-
}
756-
if stackID == "" {
757-
return
758-
}
759-
container := fmt.Sprintf("k3d-obol-stack-%s-server-0", stackID)
760-
761-
// Convert host path to the in-node path. k3d mounts $DATA_DIR → /data.
762-
relPath, err := filepath.Rel(cfg.DataDir, hostPath)
763-
if err != nil {
764-
u.Warnf("fixVolumeOwnership: cannot compute relative path from %s to %s: %v", cfg.DataDir, hostPath, err)
765-
return
766-
}
767-
if strings.HasPrefix(relPath, "..") {
768-
u.Warnf("fixVolumeOwnership: path %s is not under DataDir %s, skipping", hostPath, cfg.DataDir)
769-
return
770-
}
771-
nodePath := filepath.Join("/data", relPath)
772-
773-
cmd := exec.Command("docker", "exec", container,
774-
"chown", "-R", "1000:1000", nodePath)
775-
if err := cmd.Run(); err != nil {
776-
u.Warnf("Failed to fix volume ownership for %s: %v", nodePath, err)
786+
if err := k3dNodeExec(cfg, hostPath, "chown -R 1000:1000 {}"); err != nil {
787+
if u != nil {
788+
u.Warnf("Failed to fix volume ownership for %s: %v", hostPath, err)
789+
}
777790
}
778791
default:
779792
// k3s — direct host, try chown (succeeds if root).
780793
cmd := exec.Command("chown", "-R", "1000:1000", hostPath)
781794
if err := cmd.Run(); err != nil {
782-
u.Warnf("Failed to fix volume ownership for %s: %v (expected if not root)", hostPath, err)
795+
if u != nil {
796+
u.Warnf("Failed to fix volume ownership for %s: %v (expected if not root)", hostPath, err)
797+
}
798+
}
799+
}
800+
}
801+
802+
// ensureVolumeWritable pre-creates a host-side PVC directory and chowns it to
803+
// the current (host) user so subsequent host-side writes succeed. On k3d, the
804+
// local-path-provisioner creates directories as root inside the node container,
805+
// making them root-owned on the host. Best-effort: failures are logged but do
806+
// not block provisioning.
807+
func ensureVolumeWritable(cfg *config.Config, hostPath string, u *ui.UI) {
808+
backendName := "k3d"
809+
if data, err := os.ReadFile(filepath.Join(cfg.ConfigDir, ".stack-backend")); err == nil {
810+
backendName = strings.TrimSpace(string(data))
811+
}
812+
813+
if backendName != "k3d" {
814+
return
815+
}
816+
817+
uid := os.Getuid()
818+
gid := os.Getgid()
819+
shellCmd := fmt.Sprintf("mkdir -p {} && chown -R %d:%d {}", uid, gid)
820+
821+
if err := k3dNodeExec(cfg, hostPath, shellCmd); err != nil {
822+
if u != nil {
823+
u.Warnf("Could not pre-create volume directory %s: %v", hostPath, err)
783824
}
784825
}
785826
}
@@ -1421,6 +1462,7 @@ func SkillsSync(cfg *config.Config, id, skillsDir string, u *ui.UI) error {
14211462

14221463
u.Infof("Syncing skills from %s to volume...", skillsDir)
14231464

1465+
ensureVolumeWritable(cfg, targetDir, u)
14241466
if err := os.MkdirAll(targetDir, 0o755); err != nil {
14251467
return fmt.Errorf("failed to create skills volume directory: %w", err)
14261468
}

internal/openclaw/wallet.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,12 @@ func KeystoreVolumePath(cfg *config.Config, id string) string {
341341
// written keystore file.
342342
func provisionKeystoreToVolume(cfg *config.Config, id, keystoreID string, keystoreJSON []byte, u *ui.UI) (string, error) {
343343
dir := KeystoreVolumePath(cfg, id)
344+
345+
// On k3d, the local-path-provisioner inside the container may have already
346+
// created parent directories as root, making them root-owned on the host.
347+
// Pre-create and chown inside the k3d node so the host-side CLI can write.
348+
ensureVolumeWritable(cfg, dir, u)
349+
344350
if err := os.MkdirAll(dir, 0o700); err != nil {
345351
return "", fmt.Errorf("create keystore directory: %w", err)
346352
}
@@ -352,6 +358,7 @@ func provisionKeystoreToVolume(cfg *config.Config, id, keystoreID string, keysto
352358
return "", fmt.Errorf("write keystore: %w", err)
353359
}
354360

361+
// Re-chown to UID 1000 so the remote-signer pod can read the keystore.
355362
fixVolumeOwnership(cfg, dir, u)
356363
return path, nil
357364
}
@@ -368,6 +375,11 @@ keystorePassword:
368375
persistence:
369376
enabled: true
370377
size: 100Mi
378+
379+
# Ensure the pod's volumes are group-owned by GID 1000 so the remote-signer
380+
# process (UID 1000) can read and write the keystore PVC.
381+
podSecurityContext:
382+
fsGroup: 1000
371383
`, wallet.Password)
372384
}
373385

internal/openclaw/wallet_backup.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ func RestoreWalletCmd(cfg *config.Config, id string, opts RestoreWalletOptions,
213213

214214
// Write keystore file.
215215
keystoreDir := KeystoreVolumePath(cfg, id)
216+
ensureVolumeWritable(cfg, keystoreDir, u)
216217
if err := os.MkdirAll(keystoreDir, 0o700); err != nil {
217218
return fmt.Errorf("failed to create keystore directory: %w", err)
218219
}
@@ -381,17 +382,7 @@ func readKeystorePassword(deployDir string) (string, error) {
381382

382383
// writeKeystorePassword writes the remote-signer values YAML with the given password.
383384
func writeKeystorePassword(deployDir, password string) error {
384-
content := fmt.Sprintf(`# Remote-signer configuration
385-
# Managed by obol openclaw — do not edit manually.
386-
387-
keystorePassword:
388-
value: %q
389-
390-
persistence:
391-
enabled: true
392-
size: 100Mi
393-
`, password)
394-
385+
content := generateRemoteSignerValues(&WalletInfo{Password: password})
395386
return os.WriteFile(filepath.Join(deployDir, "values-remote-signer.yaml"), []byte(content), 0o600)
396387
}
397388

internal/openclaw/wallet_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,14 @@ func TestGenerateRemoteSignerValues(t *testing.T) {
300300
if !strings.Contains(values, "persistence:") {
301301
t.Error("values should contain persistence section")
302302
}
303+
304+
if !strings.Contains(values, "podSecurityContext:") {
305+
t.Error("values should contain podSecurityContext section")
306+
}
307+
308+
if !strings.Contains(values, "fsGroup: 1000") {
309+
t.Error("values should set fsGroup to 1000 for remote-signer PVC write access")
310+
}
303311
}
304312

305313
func TestWalletMetadataRoundTrip(t *testing.T) {

0 commit comments

Comments
 (0)