Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/main/java/io/kestra/plugin/soda/AbstractSoda.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import com.fasterxml.jackson.annotation.JsonSetter;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -185,14 +186,30 @@ private String virtualEnvCommand(RunContext runContext, Path workingDirectory, L
renderer.add("python -m venv --system-site-packages " + workingDirectory + " > /dev/null");

if (requirements != null) {
String installArgs = requirements.stream()
.map(AbstractSoda::shellQuote)
.collect(Collectors.joining(" "));

renderer.addAll(
Arrays.asList(
"./bin/pip install pip --upgrade > /dev/null",
"./bin/pip install " + runContext.render(String.join(" ", requirements)) + " > /dev/null"
"./bin/pip install " + installArgs + " > /dev/null"
)
);
}

return String.join("\n", renderer);
}

/**
* Wraps a value in single quotes for safe interpolation into a {@code /bin/sh -c} command line.
* Inside single quotes the shell treats every character literally, so all metacharacters
* ({@code ; & | > < * $ ` ( )} …) are neutralized while any valid {@code requirements.txt} syntax
* (version specifiers, extras, VCS URLs, environment markers) is preserved verbatim. The only
* character that cannot appear inside single quotes — the single quote itself — is escaped with
* the standard {@code '\''} sequence (close quote, escaped quote, reopen quote).
*/
private static String shellQuote(String value) {
return "'" + value.replace("'", "'\\''") + "'";
}
}
84 changes: 83 additions & 1 deletion src/main/java/io/kestra/plugin/soda/Scan.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import io.kestra.core.exceptions.IllegalVariableEvaluationException;
import io.kestra.core.models.annotations.Example;
Expand Down Expand Up @@ -103,6 +107,25 @@
}
)
public class Scan extends AbstractSoda implements RunnableTask<Scan.Output> {
private static final String REDACTED = "******";

/**
* Sensitive fragments matched as substrings of the normalized (alphanumeric-only, lowercased)
* key. Substring matching deliberately errs toward over-redaction — any key merely containing
* one of these (e.g. {@code keyfile}, {@code keyspace}, {@code client_secret}) is redacted — so
* that a secret is never leaked into task Output at the cost of occasionally masking a benign
* value. {@code key} already covers {@code api_key}, {@code access_key}, {@code private_key}, etc.
*/
private static final Set<String> SENSITIVE_KEY_PATTERNS = Set.of(
"password", "passwd", "pwd",
"secret",
"token",
"key",
"credential",
"accountinfojson",
"auth"
);

@Schema(
title = "SodaCL checks definition",
description = "Required map rendered to `checks.yml` and executed against the `kestra` data source. Follow SodaCL syntax; failing checks mark the task accordingly."
Expand Down Expand Up @@ -179,11 +202,70 @@ public Scan.Output run(RunContext runContext) throws Exception {
.result(scanResult)
.stdOutLineCount(output.getStdOutLineCount())
.stdErrLineCount(output.getStdOutLineCount())
.configuration(runContext.render(configuration).asMap(String.class, Object.class))
.configuration(scrubSensitiveValues(runContext.render(configuration).asMap(String.class, Object.class)))
.exitCode((Integer) output.getVars().get("exitCode"))
.build();
}

/**
* Recursively scrubs sensitive leaf values (passwords, tokens, keys, credentials, etc.) from a
* rendered configuration map before it is stored in task Output, which is persisted in execution
* state and visible to any user with read access to the execution.
*/
@SuppressWarnings("unchecked")
private static Map<String, Object> scrubSensitiveValues(Map<String, Object> map) {
Map<String, Object> scrubbed = new LinkedHashMap<>();

for (Map.Entry<String, Object> entry : map.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();

if (isSensitiveKey(key)) {
scrubbed.put(key, REDACTED);
} else {
scrubbed.put(key, scrubValue(value));
}
}

return scrubbed;
}

/**
* Recurses through container values so sensitive keys nested inside maps <em>or lists</em>
* (e.g. a list of connection maps) are scrubbed too; scalar values are returned unchanged.
*/
@SuppressWarnings("unchecked")
private static Object scrubValue(Object value) {
if (value instanceof Map) {
return scrubSensitiveValues((Map<String, Object>) value);
}

if (value instanceof List) {
List<Object> scrubbedList = new ArrayList<>();
for (Object element : (List<Object>) value) {
scrubbedList.add(scrubValue(element));
}
return scrubbedList;
}

return value;
}

private static boolean isSensitiveKey(String key) {
if (key == null) {
return false;
}

String normalized = key.toLowerCase().replaceAll("[^a-z0-9]", "");
for (String pattern : SENSITIVE_KEY_PATTERNS) {
if (normalized.contains(pattern)) {
return true;
}
}

return false;
}

protected ScanResult parseResult(RunContext runContext, ScriptOutput output) throws IOException {
ScanResult scanResult = JacksonMapper.ofJson(false).readValue(
runContext.storage().getFile(output.getOutputFiles().get("result.json")),
Expand Down
167 changes: 167 additions & 0 deletions src/test/java/io/kestra/plugin/soda/SecurityHardeningTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package io.kestra.plugin.soda;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;

/**
* Fast, dependency-free unit tests for the security-hardening helpers introduced in the
* 2026-06-30 audit remediation PR: shell-quoting of pip requirements ({@link AbstractSoda})
* and sensitive-value scrubbing of the rendered configuration ({@link Scan}). Both helpers are
* {@code private static} pure functions, so they are exercised directly via reflection rather than
* through the Docker/BigQuery integration path used by {@code ScanTest}.
*/
class SecurityHardeningTest {

private static String shellQuote(String value) throws Exception {
Method method = AbstractSoda.class.getDeclaredMethod("shellQuote", String.class);
method.setAccessible(true);
return (String) method.invoke(null, value);
}

@SuppressWarnings("unchecked")
private static Map<String, Object> scrub(Map<String, Object> input) throws Exception {
Method method = Scan.class.getDeclaredMethod("scrubSensitiveValues", Map.class);
method.setAccessible(true);
return (Map<String, Object>) method.invoke(null, input);
}

// --- shellQuote -------------------------------------------------------------------------

@Test
void shellQuote_wrapsPlainRequirement() throws Exception {
assertThat(shellQuote("soda-core-postgres"), is("'soda-core-postgres'"));
}

@Test
void shellQuote_preservesVersionOperatorsLiterally() throws Exception {
// `>` and `<` used to be interpreted as shell redirection when interpolated bare.
assertThat(shellQuote("soda-core[postgres]>=3.0,<4.0"), is("'soda-core[postgres]>=3.0,<4.0'"));
}

@Test
void shellQuote_neutralizesShellMetacharacters() throws Exception {
// Command-injection attempt: the `;` and everything after must stay inside the quotes.
String quoted = shellQuote("pkg; rm -rf /");
assertThat(quoted, is("'pkg; rm -rf /'"));
// Nothing escapes the single-quoted span, so no bare metacharacter is exposed to the shell.
assertThat(quoted.startsWith("'"), is(true));
assertThat(quoted.endsWith("'"), is(true));
}

@Test
void shellQuote_escapesEmbeddedSingleQuote() throws Exception {
// The one character that cannot live inside single quotes is escaped as '\'' .
assertThat(shellQuote("a'b"), is("'a'\\''b'"));
}

@Test
void shellQuote_preservesEnvironmentMarkers() throws Exception {
assertThat(
shellQuote("pkg; python_version >= \"3.8\""),
is("'pkg; python_version >= \"3.8\"'")
);
}

// --- scrubSensitiveValues ---------------------------------------------------------------

@Test
void scrub_redactsTopLevelSensitiveKeys() throws Exception {
Map<String, Object> input = new LinkedHashMap<>();
input.put("password", "hunter2");
input.put("host", "db.internal");

Map<String, Object> result = scrub(input);

assertThat(result.get("password"), is("******"));
assertThat(result.get("host"), is("db.internal"));
}

@Test
void scrub_recursesIntoNestedMaps() throws Exception {
Map<String, Object> connection = new LinkedHashMap<>();
connection.put("password", "s3cret");
connection.put("port", 5432);

Map<String, Object> input = new LinkedHashMap<>();
input.put("connection", connection);

Map<String, Object> result = scrub(input);

@SuppressWarnings("unchecked")
Map<String, Object> scrubbedConnection = (Map<String, Object>) result.get("connection");
assertThat(scrubbedConnection.get("password"), is("******"));
// Non-sensitive leaf values keep their original value and type.
assertThat(scrubbedConnection.get("port"), is(5432));
}

@Test
void scrub_redactsSecretKeyVariants() throws Exception {
Map<String, Object> input = new LinkedHashMap<>();
input.put("api_key", "abc");
input.put("apiKey", "abc");
input.put("token", "xyz");
input.put("account_info_json", "{...}");
input.put("client_secret", "shh");
input.put("type", "postgres");

Map<String, Object> result = scrub(input);

assertThat(result.get("api_key"), is("******"));
assertThat(result.get("apiKey"), is("******"));
assertThat(result.get("token"), is("******"));
assertThat(result.get("account_info_json"), is("******"));
assertThat(result.get("client_secret"), is("******"));
assertThat(result.get("type"), is("postgres"));
}

@Test
void scrub_recursesIntoLists() throws Exception {
// A non-sensitive key whose value is a list of maps: secrets nested in list elements
// must still be redacted (the recursion used to stop at Map values only).
Map<String, Object> node1 = new LinkedHashMap<>();
node1.put("host", "db1");
node1.put("password", "s1");
Map<String, Object> node2 = new LinkedHashMap<>();
node2.put("host", "db2");
node2.put("token", "t2");

Map<String, Object> input = new LinkedHashMap<>();
input.put("nodes", new ArrayList<>(List.of(node1, node2)));

Map<String, Object> result = scrub(input);

@SuppressWarnings("unchecked")
List<Map<String, Object>> nodes = (List<Map<String, Object>>) result.get("nodes");
assertThat(nodes, hasSize(2));
assertThat(nodes.get(0).get("password"), is("******"));
assertThat(nodes.get(0).get("host"), is("db1"));
assertThat(nodes.get(1).get("token"), is("******"));
assertThat(nodes.get(1).get("host"), is("db2"));
}

@Test
void scrub_redactsAnyKeyContainingSensitiveSubstring() throws Exception {
// Deliberate over-redaction: matching is by substring, so any key merely containing a
// sensitive fragment is masked rather than risk leaking a secret.
Map<String, Object> input = new LinkedHashMap<>();
input.put("keyfile", "/path/to/sa.json");
input.put("keyspace", "analytics");
input.put("sortkey", "created_at");
input.put("author", "alice");

Map<String, Object> result = scrub(input);

assertThat(result.get("keyfile"), is("******"));
assertThat(result.get("keyspace"), is("******"));
assertThat(result.get("sortkey"), is("******"));
assertThat(result.get("author"), is("******"));
}
}
Loading