Skip to content

Commit 797b9f0

Browse files
FizzWizZleDazzleFizzWizZleDazzle
authored
Merge pull request #9 from gluk-w/security-fixes
Fix CodeQL security vulnerabilities
2 parents d2ee1c7 + 2932133 commit 797b9f0

9 files changed

Lines changed: 62 additions & 27 deletions

File tree

‎control-plane/frontend/src/hooks/useChat.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ export function useChat(instanceId: number, enabled: boolean) {
143143
{ id: nextId(), role, content, timestamp: Date.now() },
144144
]);
145145
} else {
146-
// Log unknown events for debugging
147-
console.log("[chat] gateway event:", ev, payload);
146+
// Log unknown events for debugging (sanitize to prevent log injection)
147+
const sanitizedEv = String(ev).replace(/[\n\r]/g, ' ');
148+
console.log("[chat] gateway event:", sanitizedEv, payload);
148149
}
149150
break;
150151
}

‎control-plane/frontend/src/pages/InstanceDetailPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export default function InstanceDetailPage() {
116116
navigate(`#${tab}`, { replace: true });
117117
};
118118

119-
const [chatOpen, setChatOpen] = useState(false);
119+
const [chatOpen, _setChatOpen] = useState(false);
120120
const chatInitSentRef = useRef(false);
121121

122122
const logsHook = useInstanceLogs(instanceId, activeTab === "logs");

‎control-plane/internal/config/openclaw.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package config
22

33
import (
44
"context"
5+
"encoding/base64"
56
"encoding/json"
67
"fmt"
78
"log"
89
"strings"
910
"time"
11+
12+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
1013
)
1114

1215
// InstanceOps defines the generic primitives needed to configure an instance.
@@ -34,7 +37,7 @@ func ConfigureInstance(ctx context.Context, ops InstanceOps, name string, models
3437

3538
// Wait for instance to become running
3639
if !waitForRunning(ctx, ops, name, 120*time.Second) {
37-
log.Printf("Timed out waiting for %s to start; models/keys not configured", name)
40+
log.Printf("Timed out waiting for %s to start; models/keys not configured", logutil.SanitizeForLog(name))
3841
return
3942
}
4043

@@ -46,7 +49,7 @@ func ConfigureInstance(ctx context.Context, ops InstanceOps, name string, models
4649
}
4750
data := []byte(strings.Join(lines, "\n") + "\n")
4851
if err := ops.WriteFile(ctx, name, pathClaworcKeys, data); err != nil {
49-
log.Printf("Error writing API keys for %s: %v", name, err)
52+
log.Printf("Error writing API keys for %s: %v", logutil.SanitizeForLog(name), err)
5053
return
5154
}
5255
}
@@ -63,18 +66,20 @@ func ConfigureInstance(ctx context.Context, ops InstanceOps, name string, models
6366
}
6467
modelJSON, err := json.Marshal(modelConfig)
6568
if err != nil {
66-
log.Printf("Error marshaling model config for %s: %v", name, err)
69+
log.Printf("Error marshaling model config for %s: %v", logutil.SanitizeForLog(name), err)
6770
return
6871
}
72+
// Use base64 encoding to safely pass JSON through shell
73+
b64 := base64.StdEncoding.EncodeToString(modelJSON)
6974
cmd := []string{"su", "-", "claworc", "-c",
70-
fmt.Sprintf("openclaw config set agents.defaults.model '%s' --json", string(modelJSON))}
75+
fmt.Sprintf("openclaw config set agents.defaults.model \"$(echo '%s' | base64 -d)\" --json", b64)}
7176
_, stderr, code, err := ops.ExecInInstance(ctx, name, cmd)
7277
if err != nil {
73-
log.Printf("Error setting model config for %s: %v", name, err)
78+
log.Printf("Error setting model config for %s: %v", logutil.SanitizeForLog(name), err)
7479
return
7580
}
7681
if code != 0 {
77-
log.Printf("Failed to set model config for %s: %s", name, stderr)
82+
log.Printf("Failed to set model config for %s: %s", logutil.SanitizeForLog(name), logutil.SanitizeForLog(stderr))
7883
return
7984
}
8085
}
@@ -83,14 +88,14 @@ func ConfigureInstance(ctx context.Context, ops InstanceOps, name string, models
8388
cmd := []string{"su", "-", "claworc", "-c", "openclaw gateway stop"}
8489
_, stderr, code, err := ops.ExecInInstance(ctx, name, cmd)
8590
if err != nil {
86-
log.Printf("Error restarting gateway for %s: %v", name, err)
91+
log.Printf("Error restarting gateway for %s: %v", logutil.SanitizeForLog(name), err)
8792
return
8893
}
8994
if code != 0 {
90-
log.Printf("Failed to restart gateway for %s: %s", name, stderr)
95+
log.Printf("Failed to restart gateway for %s: %s", logutil.SanitizeForLog(name), logutil.SanitizeForLog(stderr))
9196
return
9297
}
93-
log.Printf("Models and API keys configured for %s", name)
98+
log.Printf("Models and API keys configured for %s", logutil.SanitizeForLog(name))
9499
}
95100

96101
func waitForRunning(ctx context.Context, ops InstanceOps, name string, timeout time.Duration) bool {

‎control-plane/internal/handlers/control.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/coder/websocket"
1616
"github.com/gluk-w/claworc/control-plane/internal/crypto"
1717
"github.com/gluk-w/claworc/control-plane/internal/database"
18+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
1819
"github.com/gluk-w/claworc/control-plane/internal/middleware"
1920
"github.com/gluk-w/claworc/control-plane/internal/orchestrator"
2021
"github.com/go-chi/chi/v5"
@@ -184,7 +185,7 @@ func ControlProxy(w http.ResponseWriter, r *http.Request) {
184185
targetURL += "?" + r.URL.RawQuery
185186
}
186187

187-
log.Printf("Control proxy: %s → %s", r.URL.Path, targetURL)
188+
log.Printf("Control proxy: %s → %s", logutil.SanitizeForLog(r.URL.Path), logutil.SanitizeForLog(targetURL))
188189
resp, err := getProxyClient().Get(targetURL)
189190
if err != nil {
190191
log.Printf("Control proxy error: %v", err)
@@ -261,7 +262,7 @@ func controlWSProxy(w http.ResponseWriter, r *http.Request) {
261262
}
262263
}
263264

264-
log.Printf("Control WS proxy: %s → %s", r.URL.Path, wsURL)
265+
log.Printf("Control WS proxy: %s → %s", logutil.SanitizeForLog(r.URL.Path), logutil.SanitizeForLog(wsURL))
265266
upstreamConn, _, err := websocket.Dial(dialCtx, wsURL, dialOpts)
266267
if err != nil {
267268
log.Printf("Control WS proxy: upstream dial error: %v", err)

‎control-plane/internal/handlers/files.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/gluk-w/claworc/control-plane/internal/database"
14+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
1415
"github.com/gluk-w/claworc/control-plane/internal/middleware"
1516
"github.com/gluk-w/claworc/control-plane/internal/orchestrator"
1617
"github.com/go-chi/chi/v5"
@@ -47,7 +48,7 @@ func BrowseFiles(w http.ResponseWriter, r *http.Request) {
4748

4849
entries, err := orch.ListDirectory(r.Context(), inst.Name, dirPath)
4950
if err != nil {
50-
log.Printf("Failed to list directory %s for instance %s: %v", dirPath, inst.Name, err)
51+
log.Printf("Failed to list directory %s for instance %s: %v", logutil.SanitizeForLog(dirPath), logutil.SanitizeForLog(inst.Name), err)
5152
writeError(w, http.StatusInternalServerError, fmt.Sprintf("Failed to list directory: %v", err))
5253
return
5354
}
@@ -90,7 +91,7 @@ func ReadFileContent(w http.ResponseWriter, r *http.Request) {
9091

9192
content, err := orch.ReadFile(r.Context(), inst.Name, filePath)
9293
if err != nil {
93-
log.Printf("Failed to read file %s for instance %s: %v", filePath, inst.Name, err)
94+
log.Printf("Failed to read file %s for instance %s: %v", logutil.SanitizeForLog(filePath), logutil.SanitizeForLog(inst.Name), err)
9495
writeError(w, http.StatusInternalServerError, fmt.Sprintf("Failed to read file: %v", err))
9596
return
9697
}

‎control-plane/internal/handlers/instances.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/gluk-w/claworc/control-plane/internal/config"
1717
"github.com/gluk-w/claworc/control-plane/internal/crypto"
1818
"github.com/gluk-w/claworc/control-plane/internal/database"
19+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
1920
"github.com/gluk-w/claworc/control-plane/internal/middleware"
2021
"github.com/gluk-w/claworc/control-plane/internal/orchestrator"
2122
"github.com/go-chi/chi/v5"
@@ -543,7 +544,7 @@ func CreateInstance(w http.ResponseWriter, r *http.Request) {
543544
EnvVars: envVars,
544545
})
545546
if err != nil {
546-
log.Printf("Failed to create container resources for %s: %v", name, err)
547+
log.Printf("Failed to create container resources for %s: %v", logutil.SanitizeForLog(name), err)
547548
database.DB.Model(&inst).Update("status", "error")
548549
return
549550
}
@@ -709,7 +710,7 @@ func DeleteInstance(w http.ResponseWriter, r *http.Request) {
709710

710711
if orch := orchestrator.Get(); orch != nil {
711712
if err := orch.DeleteInstance(r.Context(), inst.Name); err != nil {
712-
log.Printf("Failed to delete container resources for %s – proceeding with DB cleanup: %v", inst.Name, err)
713+
log.Printf("Failed to delete container resources for %s – proceeding with DB cleanup: %v", logutil.SanitizeForLog(inst.Name), err)
713714
}
714715
}
715716

‎control-plane/internal/logutil/sanitize.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package logutil
2+
3+
import "strings"
4+
5+
// SanitizeForLog removes newlines and control characters from user-provided
6+
// strings to prevent log injection attacks where attackers could inject
7+
// fake log entries by including newline characters.
8+
func SanitizeForLog(s string) string {
9+
// Replace all newlines with spaces
10+
s = strings.ReplaceAll(s, "\n", " ")
11+
s = strings.ReplaceAll(s, "\r", " ")
12+
// Replace tabs with spaces
13+
s = strings.ReplaceAll(s, "\t", " ")
14+
// Remove other control characters (ASCII 0-31 except space)
15+
var result strings.Builder
16+
result.Grow(len(s))
17+
for _, r := range s {
18+
if r >= 32 || r == ' ' {
19+
result.WriteRune(r)
20+
}
21+
}
22+
return result.String()
23+
}

‎control-plane/internal/orchestrator/common.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"log"
88
"strings"
99
"time"
10+
11+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
1012
)
1113

1214
const PathOpenClawConfig = "/home/claworc/.openclaw/openclaw.json"
@@ -21,29 +23,29 @@ type WaitFunc func(ctx context.Context, name string, timeout time.Duration) bool
2123

2224
func configureGatewayToken(ctx context.Context, execFn ExecFunc, name, token string, waitFn WaitFunc) {
2325
if !waitFn(ctx, name, 120*time.Second) {
24-
log.Printf("Timed out waiting for %s to start; gateway token not configured", name)
26+
log.Printf("Timed out waiting for %s to start; gateway token not configured", logutil.SanitizeForLog(name))
2527
return
2628
}
2729
cmd := []string{"su", "-", "claworc", "-c", fmt.Sprintf("openclaw config set gateway.auth.token %s", token)}
2830
_, stderr, code, err := execFn(ctx, name, cmd)
2931
if err != nil {
30-
log.Printf("Error configuring gateway token for %s: %v", name, err)
32+
log.Printf("Error configuring gateway token for %s: %v", logutil.SanitizeForLog(name), err)
3133
return
3234
}
3335
if code != 0 {
34-
log.Printf("Failed to configure gateway token for %s: %s", name, stderr)
36+
log.Printf("Failed to configure gateway token for %s: %s", logutil.SanitizeForLog(name), logutil.SanitizeForLog(stderr))
3537
return
3638
}
3739
_, stderr, code, err = execFn(ctx, name, cmdGatewayStop)
3840
if err != nil {
39-
log.Printf("Error restarting gateway for %s: %v", name, err)
41+
log.Printf("Error restarting gateway for %s: %v", logutil.SanitizeForLog(name), err)
4042
return
4143
}
4244
if code != 0 {
43-
log.Printf("Failed to restart gateway for %s: %s", name, stderr)
45+
log.Printf("Failed to restart gateway for %s: %s", logutil.SanitizeForLog(name), logutil.SanitizeForLog(stderr))
4446
return
4547
}
46-
log.Printf("Gateway token configured for %s", name)
48+
log.Printf("Gateway token configured for %s", logutil.SanitizeForLog(name))
4749
}
4850

4951
func updateInstanceConfig(ctx context.Context, execFn ExecFunc, name string, configJSON string) error {

‎control-plane/internal/orchestrator/docker.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
dockerclient "github.com/docker/docker/client"
1818
"github.com/docker/go-units"
1919
"github.com/gluk-w/claworc/control-plane/internal/config"
20+
"github.com/gluk-w/claworc/control-plane/internal/logutil"
2021
)
2122

2223
const (
@@ -156,7 +157,7 @@ func (d *DockerOrchestrator) CreateInstance(ctx context.Context, params CreatePa
156157
Labels: map[string]string{"managed-by": labelManagedBy, "instance": params.Name},
157158
})
158159
if err != nil {
159-
log.Printf("Volume %s may already exist: %v", volName, err)
160+
log.Printf("Volume %s may already exist: %v", logutil.SanitizeForLog(volName), err)
160161
}
161162
}
162163

@@ -316,14 +317,14 @@ func (d *DockerOrchestrator) DeleteInstance(ctx context.Context, name string) er
316317
// Remove container
317318
err := d.client.ContainerRemove(ctx, name, container.RemoveOptions{Force: true})
318319
if err != nil && !dockerclient.IsErrNotFound(err) {
319-
log.Printf("Remove container %s: %v", name, err)
320+
log.Printf("Remove container %s: %v", logutil.SanitizeForLog(name), err)
320321
}
321322

322323
// Remove volumes
323324
for _, suffix := range volumeSuffixes {
324325
volName := d.volumeName(name, suffix)
325326
if err := d.client.VolumeRemove(ctx, volName, true); err != nil && !dockerclient.IsErrNotFound(err) {
326-
log.Printf("Remove volume %s: %v", volName, err)
327+
log.Printf("Remove volume %s: %v", logutil.SanitizeForLog(volName), err)
327328
}
328329
}
329330
return nil

0 commit comments

Comments
 (0)