Skip to content

Commit

Permalink
Merge pull request #3144 from mercedes-benz/feature-3142-reduce-env-v…
Browse files Browse the repository at this point in the history
…isiblitiy-pds-scripts

Introduce PDS environment cleanup for caller scripts #3142
  • Loading branch information
de-jcup committed May 24, 2024
2 parents f2ffc93 + ecc1296 commit aa296df
Show file tree
Hide file tree
Showing 13 changed files with 468 additions and 4 deletions.
11 changes: 11 additions & 0 deletions sechub-integrationtest/integrationtest-pds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ PDS_DEFAULT_PORT=8444
PDS_DEFAULT_VERSION="0.0.0"
PDS_DEFAULT_TEMPFOLDER="temp-shared"

# --------------------------------------------------------------------------------
# Export special variables to test script environment cleanup works as expected
# - we just start the PDS with those variables
# - only INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED is whitelisted and must be available
# inside integration tests variable dump, the forbidden one not, because not
# whitelisted...
# --------------------------------------------------------------------------------
export INTEGRATIONTEST_PDS_STARTED_BY_SCRIPT="true" # is checked in integration tests to check if server started by script
export INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED="accepted"
export INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN="forbidden"

function log() {
echo "$1"
echo "`date +%Y-%m-%d\ %H:%M:%S` $1" >> "$LOGFILE"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ fi
if [[ "$PDS_TEST_KEY_VARIANTNAME" = "j" ]]; then
dumpVariable "TEST_MAPPING1_REPLACE_PROJECT1"
dumpVariable "TEST_MAPPING2_NOT_EXISTING_IN_SECHUB"

# we dump here for PDSUseSecHubCentralMappingInJobScenario16IntTest to check that
# environment variables which are not white listed are not available here
dumpVariable "INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED" # is white listed variable - must be not empty
dumpVariable "INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN" # not white listed - must be empty in test
dumpVariable "PATH" # check that this as a default white listed environment variable is available
fi

#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,12 @@ public static List<UUID> fetchAllPDSJobUUIDsForSecHubJob(UUID sechubJobUUID) {
return jobUUIDS;
}

public static String getPDSServerEnvironmentVariableValue(String environmentVariableName) {
String url = getPDSURLBuilder().buildIntegrationTestFetchEnvironmentVariableValue(environmentVariableName);
String value = getPDSAdminRestHelper().getStringFromURL(url);
return value;
}

public static void dumpAllPDSJobOutputsForSecHubJob(UUID sechubJobUUID) {
dumpAllPDSJobOutputsForSecHubJob(sechubJobUUID, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,37 @@
import static com.mercedesbenz.sechub.integrationtest.api.IntegrationTestMockMode.*;
import static com.mercedesbenz.sechub.integrationtest.api.TestAPI.*;
import static com.mercedesbenz.sechub.integrationtest.scenario16.Scenario16.*;
import static org.junit.Assert.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.Map;
import java.util.UUID;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.Timeout;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.mercedesbenz.sechub.commons.mapping.MappingData;
import com.mercedesbenz.sechub.commons.mapping.MappingEntry;
import com.mercedesbenz.sechub.integrationtest.api.IntegrationTestSetup;
import com.mercedesbenz.sechub.integrationtest.api.TestAPI;
import com.mercedesbenz.sechub.integrationtest.api.TestProject;
import com.mercedesbenz.sechub.integrationtest.internal.IntegrationTestExampleConstants;
import com.mercedesbenz.sechub.integrationtest.scenario22.PDSPrepareIntegrationScenario22IntTest;

public class PDSUseSecHubCentralMappingInJobScenario16IntTest {

private static final String TRUE = "true";
private static final String FORBIDDEN = "forbidden";
private static final String ACCEPTED = "accepted";
private static final String INTEGRATIONTEST_PDS_STARTED_BY_SCRIPT = "INTEGRATIONTEST_PDS_STARTED_BY_SCRIPT";
private static final String INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED = "INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED";
private static final String INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN = "INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN";

private static final Logger LOG = LoggerFactory.getLogger(PDSPrepareIntegrationScenario22IntTest.class);

@Rule
public IntegrationTestSetup setup = IntegrationTestSetup.forScenario(Scenario16.class);

Expand Down Expand Up @@ -54,9 +69,42 @@ public void pds_script_does_have_the_mappings_from_sechub_injected_as_environmen
String expectedMapping2Json = "{}";

// check the script has the mappings injected :
assertPDSJob(TestAPI.assertAndFetchPDSJobUUIDForSecHubJob(jobUUID)).
UUID pdsJobUUID = TestAPI.assertAndFetchPDSJobUUIDForSecHubJob(jobUUID);
assertPDSJob(pdsJobUUID).
containsVariableTestOutput(IntegrationTestExampleConstants.PDS_ENV_NAME_MAPPING_ID_1_REPLACE_ANY_PROJECT1, expectedMapping1Json).
containsVariableTestOutput(IntegrationTestExampleConstants.PDS_ENV_NAME_MAPPING_ID_2_NOT_EXISTING_IN_SECHUB, expectedMapping2Json);

// additional test: here we test that the script environment has only white listed parts from
// parent process (the variables were defined and exported to PDS on startup by integrationtest-pds.sh)
String pdsStartedByScriptValue = TestAPI.getPDSServerEnvironmentVariableValue(INTEGRATIONTEST_PDS_STARTED_BY_SCRIPT);
if (TRUE.equals(pdsStartedByScriptValue)) {
Map<String, String> variables = fetchPDSVariableTestOutputMap(pdsJobUUID);

// precondition check
assertEquals(ACCEPTED, TestAPI.getPDSServerEnvironmentVariableValue(INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED));
assertEquals(FORBIDDEN, TestAPI.getPDSServerEnvironmentVariableValue(INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN));

assertEquals(ACCEPTED, variables.get(INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED)); // defined + white listed
assertEquals("", variables.get(INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN));// was defined, but not white listed, means dump returns empty

assertNotNull(variables.get("PATH")); // one of the default PDS white list entries for script environments

}else {
LOG.error("#".repeat(120));
LOG.error("### ERROR - local PDS server cannot be tested without environment variables set!");
LOG.error("#".repeat(120));
LOG.error("The integration test usese a PDS server which is running not from script but locally (from an IDE).");
LOG.error("Means the environment variables are not set on PDS startup process and cannot be tested!");
LOG.error("The test will just skip the pds script cleanup test part in this case because otherwise always failing");
LOG.error("");
LOG.error("If you want to test this locally,you have to set the env variables on PDS start locally:");
LOG.error(" {}={} ",INTEGRATIONTEST_PDS_STARTED_BY_SCRIPT, TRUE);
LOG.error(" {}={} ",INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED, ACCEPTED);
LOG.error(" {}={} ",INTEGRATIONTEST_SCRIPT_ENV_FORBIDDEN, FORBIDDEN);
LOG.error("#".repeat(120));
}


/* @formatter:on */
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ public String getWorkspaceUploadFolder(@PathVariable("jobUUID") UUID pdsJobUUID)
return uploadFolder;
}

@RequestMapping(path = PDSAPIConstants.API_ANONYMOUS + "integrationtest/env/{envVariable}", method = RequestMethod.GET, produces = {
MediaType.TEXT_PLAIN_VALUE })
public String getEnvironmentVariableValue(@PathVariable("envVariable") String envVariable) {
String value = System.getenv(envVariable);

LOG.info("Integration test checks environment variable:{} - value:{}", envVariable, value);
return value;
}

@RequestMapping(path = PDSAPIConstants.API_ANONYMOUS + "integrationtest/last/started/job/uuid", method = RequestMethod.GET, produces = {
MediaType.APPLICATION_JSON_VALUE })
public String fetchLastStartedPDSJobUUID() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// SPDX-License-Identifier: MIT
package com.mercedesbenz.sechub.pds.execution;

/**
* These enumeration represents variable names which are white listed per
* default by {@link PDSScriptEnvironmentCleaner#clean(java.util.Map)}.
*
* @author Albert Tregnaghi
*
*/
public enum PDSDefaulScriptEnvironmentVariableWhitelist {

HOME,

HOSTNAME,

PATH,

PWD,

TERM,

UID,

USER,

;

}
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,12 @@ private void createProcess(UUID jobUUID, PDSJobConfiguration config, String path
* job was marked as ready to start
*/

PDSExecutionEnvironmentService environmentService = serviceCollection.getEnvironmentService();
Map<String, String> buildEnvironmentMap = environmentService.buildEnvironmentMap(config);
PDSExecutionEnvironmentService environmentService = getEnvironmentService();

Map<String, String> environment = builder.environment();
environmentService.removeAllNonWhitelistedEnvironmentVariables(environment);

Map<String, String> buildEnvironmentMap = environmentService.buildEnvironmentMap(config);
environment.putAll(buildEnvironmentMap);

WorkspaceLocationData locationData = workspaceService.createLocationData(jobUUID);
Expand Down Expand Up @@ -580,6 +583,10 @@ private PDSJobTransactionService getJobTransactionService() {
return serviceCollection.getJobTransactionService();
}

private PDSExecutionEnvironmentService getEnvironmentService() {
return serviceCollection.getEnvironmentService();
}

private PDSCheckJobStatusService getJobStatusService() {
return serviceCollection.getJobStatusService();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;

import com.mercedesbenz.sechub.commons.pds.ExecutionPDSKey;
import com.mercedesbenz.sechub.commons.pds.PDSConfigDataKeyProvider;
import com.mercedesbenz.sechub.commons.pds.PDSLauncherScriptEnvironmentConstants;
import com.mercedesbenz.sechub.pds.PDSMustBeDocumented;
import com.mercedesbenz.sechub.pds.commons.core.config.PDSProductParameterDefinition;
import com.mercedesbenz.sechub.pds.commons.core.config.PDSProductParameterSetup;
import com.mercedesbenz.sechub.pds.commons.core.config.PDSProductSetup;
import com.mercedesbenz.sechub.pds.config.PDSServerConfigurationService;
import com.mercedesbenz.sechub.pds.job.PDSJobConfiguration;

import jakarta.annotation.PostConstruct;

@Service
public class PDSExecutionEnvironmentService {

Expand All @@ -33,6 +37,26 @@ public class PDSExecutionEnvironmentService {
@Autowired
PDSServerConfigurationService serverConfigService;

@PDSMustBeDocumented(value = "A comma separated list of environment variable names which are white listed from PDS script environment cleanup. "
+ "Entries can also end with an asterisk, in this case every variable name starting with this entry will be whitelisted (e.g. PDS_STORAGE_* will whitelist PDS_STORAGE_S3_USER etc.)\n\n"
+ "The cleanup process prevents inheritage of environment variables from PDS parent process. "
+ "There are some default whitelist entries which are automatically added (e.g. HOME, PATH, ..) "
+ "but additional whitelist entries are added by this property/environment entry on PDS startup.", scope = "execution")
@Value("${pds.script.env.whitelist:}")
String pdsScriptEnvWhitelist;

PDSScriptEnvironmentCleaner cleaner = new PDSScriptEnvironmentCleaner();

@PostConstruct
void postConstruct() {
LOG.info("PDS script environment variable white list: '{}'", pdsScriptEnvWhitelist);
cleaner.setWhiteListCommaSeparated(pdsScriptEnvWhitelist);
}

public void removeAllNonWhitelistedEnvironmentVariables(Map<String, String> environment) {
cleaner.clean(environment);
}

public Map<String, String> buildEnvironmentMap(PDSJobConfiguration config) {
Map<String, String> map = new LinkedHashMap<>();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// SPDX-License-Identifier: MIT
package com.mercedesbenz.sechub.pds.execution;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;

public class PDSScriptEnvironmentCleaner {

private Set<String> whiteList = new HashSet<>();

/**
* Removes all environment variables from given environment map, except default
* and explicit white listed variable names. Default variable names are inside
* {@link PDSDefaulScriptEnvironmentVariableWhitelist}, set explicit white
* listed names are set via {@link #setWhiteListCommaSeparated(String)}
*
* @param environment the environment map to clean
*/
public void clean(Map<String, String> environment) {
/* create backup */
Map<String, String> backup = new HashMap<>();
backup.putAll(environment);

/* initial clear all */
environment.clear();

/* copy white listed parts back to environment */
Iterator<String> variableNameIt = backup.keySet().iterator();

while (variableNameIt.hasNext()) {
String variableName = variableNameIt.next();
if (isWhitelistedEnvironmentVariable(variableName)) {
String backupValue = backup.get(variableName);
environment.put(variableName, backupValue);
}
}

}

/**
* Sets the white list as a comma separated list of variable names to exclude
* from cleaning. If a variable name ends with an asterisk every variable which
* begins with such prefix will be accepted. For example: PDS_STORAGE_* will
* white list any kind of environment variable which starts with PDS_STORAGE_
* (e.g. PDS_STORAGE_S3_USER).
*
* @param commaSeparatedWhiteList comma separated list of white list entries.
*/
public void setWhiteListCommaSeparated(String commaSeparatedWhiteList) {
whiteList.clear();

if (commaSeparatedWhiteList == null || commaSeparatedWhiteList.isBlank()) {
return;
}

String[] splitted = commaSeparatedWhiteList.split(",");
for (String whiteListEntry : splitted) {
String trimmedWhiteListEntry = whiteListEntry.trim();
if (trimmedWhiteListEntry.isBlank()) {
continue;
}
whiteList.add(trimmedWhiteListEntry);
}
}

private boolean isWhitelistedEnvironmentVariable(String variableName) {
if (variableName == null) {
return false;
}

/* handle default white list entries */
for (PDSDefaulScriptEnvironmentVariableWhitelist defaultWhitelistVariable : PDSDefaulScriptEnvironmentVariableWhitelist.values()) {
if (defaultWhitelistVariable.name().equals(variableName)) {
return true;
}
}

/* handle explicit white list entries */
if (whiteList.contains(variableName)) {
return true;
}

/* handle asterisk variants */
for (String whiteListEntry : whiteList) {
int length = whiteListEntry.length();

if (whiteListEntry.endsWith("*") && length > 2) {
String prefix = whiteListEntry.substring(0, length - 1);
if (variableName.startsWith(prefix)) {
return true;
}
}
}

/* not white listed */
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ pds:
admin:
userid: pds-inttest-admin
apitoken: '{noop}pds-inttest-apitoken'


script:
env:
whitelist: 'PDS_STORAGE_*,INTEGRATIONTEST_SCRIPT_ENV_ACCEPTED'
logging:
level:
com.mercedesbenz.sechub: DEBUG
Expand Down
Loading

0 comments on commit aa296df

Please sign in to comment.