Skip to content

Commit 459fdff

Browse files
SecretService and Provider, filter out secret environment variables (#7671)
#### Rationale We want to make it easy to identify configuration that's considered secret, like API keys or other credentials. #### Changes - Scrub sensitive environment variables for reporting in Admin Console and forking external processes - Pull secrets from startup properties or environment variables - Adopt `ProcessBuilder` variant that does the scrubbing - Admin Console reporting for secrets and their sources (not their values) --------- Co-authored-by: labkey-jeckels <jeckels@labkey.com>
1 parent b6b55ce commit 459fdff

16 files changed

Lines changed: 666 additions & 53 deletions

api/src/org/labkey/api/module/ModuleLoader.java

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2561,67 +2561,65 @@ public FileLike getStartupPropDirectory()
25612561
private void loadStartupProps()
25622562
{
25632563
FileLike propsDir = getStartupPropDirectory();
2564-
if (null == propsDir)
2565-
return;
2566-
2567-
if (!propsDir.isDirectory())
2568-
return;
25692564

2570-
FileLike newinstall = propsDir.resolveChild("newinstall");
2571-
if (newinstall.isFile())
2565+
if (null != propsDir && propsDir.isDirectory())
25722566
{
2573-
_log.debug("'newinstall' file detected: {}", newinstall.toNioPathForRead());
2567+
FileLike newinstall = propsDir.resolveChild("newinstall");
2568+
if (newinstall.isFile())
2569+
{
2570+
_log.debug("'newinstall' file detected: {}", newinstall.toNioPathForRead());
25742571

2575-
_newInstall = true;
2572+
_newInstall = true;
25762573

2577-
// propsDir is readonly, so we need to cheat to get a File
2578-
var newInstallFile = newinstall.toNioPathForRead().toFile();
2579-
if (newInstallFile.canWrite())
2580-
newInstallFile.delete();
2574+
// propsDir is readonly, so we need to cheat to get a File
2575+
var newInstallFile = newinstall.toNioPathForRead().toFile();
2576+
if (newInstallFile.canWrite())
2577+
newInstallFile.delete();
2578+
else
2579+
throw new ConfigurationException("file 'newinstall' exists, but is not writeable: " + newinstall.toNioPathForRead());
2580+
}
25812581
else
2582-
throw new ConfigurationException("file 'newinstall' exists, but is not writeable: " + newinstall.toNioPathForRead());
2583-
}
2584-
else
2585-
{
2586-
_log.debug("no 'newinstall' file detected");
2587-
}
2588-
2589-
List<FileLike> propFiles = propsDir.getChildren().stream().filter(f -> f.getName().endsWith(".properties")).toList();
2582+
{
2583+
_log.debug("no 'newinstall' file detected");
2584+
}
25902585

2591-
if (!propFiles.isEmpty())
2592-
{
2593-
List<FileLike> sortedPropFiles = propFiles.stream()
2594-
.sorted(Comparator.comparing(FileLike::getName).reversed())
2595-
.toList();
2586+
List<FileLike> propFiles = propsDir.getChildren().stream().filter(f -> f.getName().endsWith(".properties")).toList();
25962587

2597-
for (FileLike propFile : sortedPropFiles)
2588+
if (!propFiles.isEmpty())
25982589
{
2599-
_log.debug("loading propsFile: {}", propFile.toNioPathForRead());
2590+
List<FileLike> sortedPropFiles = propFiles.stream()
2591+
.sorted(Comparator.comparing(FileLike::getName).reversed())
2592+
.toList();
26002593

2601-
try (InputStream in = propFile.openInputStream())
2594+
for (FileLike propFile : sortedPropFiles)
26022595
{
2603-
Properties props = new Properties();
2604-
props.load(in);
2596+
_log.debug("loading propsFile: {}", propFile.toNioPathForRead());
26052597

2606-
for (Map.Entry<Object, Object> entry : props.entrySet())
2598+
try (InputStream in = propFile.openInputStream())
26072599
{
2608-
if (entry.getKey() instanceof String && entry.getValue() instanceof String)
2600+
Properties props = new Properties();
2601+
props.load(in);
2602+
2603+
for (Map.Entry<Object, Object> entry : props.entrySet())
26092604
{
2610-
_log.trace("property '{}' resolved to value: '{}'", entry.getKey(), entry.getValue());
2605+
if (entry.getKey() instanceof String && entry.getValue() instanceof String)
2606+
{
2607+
_log.trace("property '{}' resolved to value: '{}'", entry.getKey(), entry.getValue());
26112608

2612-
addStartupPropertyEntry(entry.getKey().toString(), entry.getValue().toString());
2609+
addStartupPropertyEntry(entry.getKey().toString(), entry.getValue().toString());
2610+
}
26132611
}
26142612
}
2615-
}
2616-
catch (Exception e)
2617-
{
2618-
_log.error("Error parsing startup config properties file '{}'", propFile.toNioPathForRead(), e);
2613+
catch (Exception e)
2614+
{
2615+
_log.error("Error parsing startup config properties file '{}'", propFile.toNioPathForRead(), e);
2616+
}
26192617
}
26202618
}
2621-
}
2622-
else
2623-
{
2624-
_log.debug("no propFiles to load");
2619+
else
2620+
{
2621+
_log.debug("no propFiles to load");
2622+
}
26252623
}
26262624

26272625
// load any system properties with the labkey prop prefix

api/src/org/labkey/api/reports/ExternalScriptEngine.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.labkey.api.reader.Readers;
2626
import org.labkey.api.reports.report.r.ParamReplacementSvc;
2727
import org.labkey.api.util.ExceptionUtil;
28+
import org.labkey.api.util.LabKeyProcessBuilder;
2829
import org.labkey.api.util.FileUtil;
2930
import org.labkey.api.util.QuietCloser;
3031
import org.labkey.api.util.URIUtil;
@@ -126,7 +127,7 @@ public Object eval(String script, ScriptContext context) throws ScriptException
126127
protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptException
127128
{
128129
String[] params = formatCommand(scriptFile, context);
129-
ProcessBuilder pb = new ProcessBuilder(params);
130+
LabKeyProcessBuilder pb = new LabKeyProcessBuilder(params);
130131
pb = pb.directory(getWorkingDir(context).toNioPathForRead().toFile());
131132

132133
final long timeout = getTimeout(context);
@@ -318,7 +319,7 @@ else if (value.startsWith("\""))
318319
* Execute the external script engine in separate process
319320
* @return the exit code for the invocation - 0 if the process completed successfully.
320321
*/
321-
protected int runProcess(ScriptContext context, ProcessBuilder pb, StringBuffer output, long timeout, TimeUnit timeoutUnit)
322+
protected int runProcess(ScriptContext context, LabKeyProcessBuilder pb, StringBuffer output, long timeout, TimeUnit timeoutUnit)
322323
{
323324
Process proc;
324325
try
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package org.labkey.api.secrets;
2+
3+
import org.labkey.api.settings.StartupProperty;
4+
5+
/**
6+
* Describes a named secret that a module needs to access. Register instances with
7+
* {@link SecretService#register} during module startup; retrieve values via
8+
* {@link SecretService#getSecret}.
9+
*
10+
* Startup property file convention: {@code secret.<propertyName>=<value>}
11+
* Java property convention: {@code -Plabkey.prop.secret.<propertyName>=<value>}
12+
* Environment variable convention: {@code export <propertyName>=<value>}
13+
*/
14+
public class SecretProperty implements StartupProperty
15+
{
16+
private final String _name;
17+
private final String _description;
18+
19+
public SecretProperty(String name)
20+
{
21+
this(name, "Secret: " + name);
22+
}
23+
24+
public SecretProperty(String name, String description)
25+
{
26+
_name = name;
27+
_description = description;
28+
}
29+
30+
@Override
31+
public String getPropertyName()
32+
{
33+
return _name;
34+
}
35+
36+
@Override
37+
public String getDescription()
38+
{
39+
return _description;
40+
}
41+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package org.labkey.api.secrets;
2+
3+
import org.jetbrains.annotations.NotNull;
4+
import org.jetbrains.annotations.Nullable;
5+
6+
/**
7+
* SPI for a secret source. Implementations cover built-in sources (startup property files,
8+
* environment variables) and external stores (e.g., AWS SSM Parameter Store).
9+
* Providers are consulted in priority order by {@link SecretService}.
10+
*/
11+
public interface SecretProvider
12+
{
13+
/** Returns the secret for the given property name, or null if not available from this source. */
14+
@Nullable String getSecret(String propertyName);
15+
16+
/** Human-readable name for this source, shown on the admin secrets page. */
17+
@NotNull String getDescription();
18+
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package org.labkey.api.secrets;
2+
3+
import org.jetbrains.annotations.NotNull;
4+
import org.jetbrains.annotations.Nullable;
5+
import org.labkey.api.services.ServiceRegistry;
6+
7+
import java.util.List;
8+
9+
/**
10+
* Internal service that provides access to secrets (API keys, passwords, etc.) without
11+
* requiring callers to know where the secret is stored. Secrets may come from startup
12+
* property files, process environment variables, or an external store such as AWS SSM.
13+
*
14+
* <p>Modules should:
15+
* <ol>
16+
* <li>Declare each secret as a {@code static final SecretProperty} constant.</li>
17+
* <li>Call {@link #register} in their {@code doStartup()} method.</li>
18+
* <li>Call {@link #getSecret} wherever the value is needed.</li>
19+
* </ol>
20+
*
21+
* <p>Startup property file convention: {@code secret.<propertyName>=<value>}
22+
*/
23+
public interface SecretService
24+
{
25+
static @NotNull SecretService get()
26+
{
27+
SecretService svc = ServiceRegistry.get().getService(SecretService.class);
28+
if (svc == null)
29+
throw new IllegalStateException("SecretService has not been initialized");
30+
return svc;
31+
}
32+
33+
static void setInstance(SecretService service)
34+
{
35+
ServiceRegistry.get().registerService(SecretService.class, service);
36+
}
37+
38+
/**
39+
* Declare that the calling module may request the named secret. Should be called
40+
* from {@code Module.doStartup()}. Registration is for documentation and filtering
41+
* (e.g., admin env-var page redaction); it does not affect whether a value is returned.
42+
*/
43+
void register(@NotNull SecretProperty property);
44+
45+
/**
46+
* Retrieve the value of a secret. Returns {@code null} if the secret has not been
47+
* configured in any source. Never logs or caches the returned value.
48+
*
49+
* <p><strong>Identity contract:</strong> the {@code property} argument must be the exact
50+
* {@code static final} instance that was passed to {@link #register}. A freshly constructed
51+
* {@code new SecretProperty("SOME_KEY")} will always return {@code null}, even if a secret
52+
* with that name is configured. This prevents unregistered callers from reading secrets
53+
* they did not declare.
54+
*/
55+
@Nullable String getSecret(@NotNull SecretProperty property);
56+
57+
/** Returns true if the given property name has been registered via {@link #register}. */
58+
boolean isRegisteredSecret(@NotNull String name);
59+
60+
/**
61+
* Register a high-priority {@link SecretProvider} (e.g., AWS SSM). This provider is
62+
* consulted before the built-in startup-property and environment-variable providers.
63+
*/
64+
void setExternalProvider(@NotNull SecretProvider provider);
65+
66+
/**
67+
* Returns read-only status for every registered secret, sorted by name.
68+
* Never includes secret values — safe to display in admin UI.
69+
*/
70+
@NotNull List<SecretStatus> getSecretStatuses();
71+
72+
/**
73+
* Returns a human-readable description of the active external provider (e.g.,
74+
* "AWS SSM Parameter Store"), or {@code null} if no external provider is registered.
75+
* The external provider takes priority over startup-property and environment-variable sources.
76+
*/
77+
@Nullable String getExternalProviderDescription();
78+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package org.labkey.api.secrets;
2+
3+
import org.jetbrains.annotations.Nullable;
4+
5+
/**
6+
* Read-only status of a registered secret — suitable for admin UI display.
7+
* Never contains the secret value itself.
8+
*
9+
* @param source description of the provider that holds this secret
10+
* (e.g. "Startup property file", "Environment variable"), or
11+
* {@code null} if no provider has a value for it
12+
*/
13+
public record SecretStatus(String name, String description, @Nullable String source)
14+
{
15+
public boolean isSet()
16+
{
17+
return source != null;
18+
}
19+
}

api/src/org/labkey/api/util/GUID.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ private static String networkIdentifier()
126126

127127
try
128128
{
129-
ProcessBuilder cmd = new ProcessBuilder("ipconfig.exe", "/all");
129+
LabKeyProcessBuilder cmd = new LabKeyProcessBuilder("ipconfig.exe", "/all");
130130
cmd.redirectErrorStream(true);
131131
p = cmd.start();
132132
}
@@ -135,7 +135,7 @@ private static String networkIdentifier()
135135
{
136136
try
137137
{
138-
ProcessBuilder cmd = new ProcessBuilder("ifconfig", "-a");
138+
LabKeyProcessBuilder cmd = new LabKeyProcessBuilder("ifconfig", "-a");
139139
cmd.redirectErrorStream(true);
140140
p = cmd.start();
141141
}

0 commit comments

Comments
 (0)