From a106115dd76826d9338fb6e56aa59c760087d769 Mon Sep 17 00:00:00 2001
From: Davide Principi <davide.principi@nethesis.it>
Date: Tue, 7 Nov 2023 11:09:42 +0100
Subject: [PATCH] agent. Fix secrets obfuscation (#515)

The flatten/unflatten library has a known bug that does not preserve
JSON arrays. This fix recursively walk the JSON decoded structure and
replaces sensitive fields in the same way.
---
 core/agent/htask.go                           | 60 ++++++++++++++-----
 .../test/suite/10__obfuscate_input.robot      |  9 +++
 2 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/core/agent/htask.go b/core/agent/htask.go
index 76da6e7ff..553114541 100644
--- a/core/agent/htask.go
+++ b/core/agent/htask.go
@@ -36,7 +36,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/nqd/flat"
 	"github.com/NethServer/ns8-core/core/agent/models"
 	"github.com/NethServer/ns8-core/core/agent/validation"
 	"github.com/go-redis/redis/v8"
@@ -402,21 +401,54 @@ func publishStatus(client redis.Cmdable, progressChannel string, actionDescripto
 	}
 }
 
-func obscureTaskInput(payload string) string {
-	var jsonDyn map[string]interface{}
-	json.Unmarshal([]byte(payload), &jsonDyn)
-	flattenedInput, _ := flat.Flatten(jsonDyn, nil)
+func obscureTaskInput(jsonStr string) string {
+	var jsonData map[string]interface{}
+	if err := json.Unmarshal([]byte(jsonStr), &jsonData); err != nil {
+		log.Println(SD_ERR+"Error unmarshalling JSON:", err)
+		return jsonStr
+	}
+
+	recursiveObscureSensitiveKeys(jsonData)
+
+	updatedJson, err := json.Marshal(jsonData)
+	if err != nil {
+		log.Println(SD_ERR+"Error marshalling JSON:", err)
+		return jsonStr
+	}
+
+	return string(updatedJson)
+}
+
+func isSensitive(target string) bool {
 	sensitiveList := []string{"password", "secret", "token"}
-	// search for sensitve data, in sensitive list
-	for k, _ := range flattenedInput {
-		for _, s := range sensitiveList {
-			if strings.HasPrefix(k, "data.") && strings.HasSuffix("." + strings.ToLower(k), strings.ToLower(s)) {
-				flattenedInput[k] = "XXX"
+	ltarget := strings.ToLower(target)
+	for _, sensitive := range sensitiveList {
+		if strings.HasSuffix(ltarget, sensitive) {
+			return true
+		}
+	}
+	return false
+}
+
+func recursiveObscureSensitiveKeys(data interface{}) {
+	switch v := data.(type) {
+	case map[string]interface{}:
+		// It's an object, so iterate through its key-value pairs
+		for key, value := range v {
+			// Recursively update the value
+			recursiveObscureSensitiveKeys(value)
+
+			// Check for sensitive keys
+			if isSensitive(key) {
+				v[key] = "XXX" // replace the secret value
 			}
 		}
+
+	case []interface{}:
+		// It's an array, so iterate through its elements
+		for _, item := range v {
+			// Recursively update the element
+			recursiveObscureSensitiveKeys(item)
+		}
 	}
-	obscuredTask, _ := flat.Unflatten(flattenedInput, nil)
-	// convert to JSON string
-	taskJson, _ := json.Marshal(obscuredTask)
-	return string(taskJson[:])
 }
diff --git a/core/agent/test/suite/10__obfuscate_input.robot b/core/agent/test/suite/10__obfuscate_input.robot
index 810e6b34b..67d8ce60e 100644
--- a/core/agent/test/suite/10__obfuscate_input.robot
+++ b/core/agent/test/suite/10__obfuscate_input.robot
@@ -11,3 +11,12 @@ Passwords are obfuscated in Redis context keys
     When The command is received   set  obfuscate-test/context
     Then The task context should contain    XXX
     And The task context should contain    PRE-SERVED
+
+Obfuscation does not alter the input structure
+    Given The task is submitted    obfuscate-test    {"tags":["t1","t2"],"account_info":{"password":"Nethesis,1234","claims":["c1","c2"],"ratio":1.2},"password_not_replaced":"PRE-SERVED"}
+    When The command is received   set  obfuscate-test/context
+    Then The task context should contain    XXX
+    And The task context should contain    PRE-SERVED
+    And The task context should contain    ["t1","t2"]
+    And The task context should contain    ["c1","c2"]
+    And The task context should contain    1.2