From c59da11c25e7b5de43eef613344e16aa6a780dea Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Thu, 14 May 2026 18:51:42 -0700 Subject: [PATCH 1/9] checkpoint initial implementation of service interface SecretService --- .../org/labkey/api/module/ModuleLoader.java | 84 ++++---- .../api/reports/ExternalScriptEngine.java | 5 +- .../api/secrets/ExternalSecretProvider.java | 20 ++ .../labkey/api/secrets/SecretProperty.java | 40 ++++ .../org/labkey/api/secrets/SecretService.java | 58 ++++++ api/src/org/labkey/api/util/GUID.java | 4 +- .../labkey/api/util/LabKeyProcessBuilder.java | 96 ++++++++++ core/src/org/labkey/core/CoreModule.java | 8 + .../labkey/core/admin/AdminController.java | 9 +- .../core/secrets/SecretServiceImpl.java | 180 ++++++++++++++++++ .../org/labkey/core/vcs/VcsServiceImpl.java | 3 +- .../pipeline/analysis/CommandTaskImpl.java | 7 +- .../pipeline/cluster/ClusterStartup.java | 3 +- 13 files changed, 464 insertions(+), 53 deletions(-) create mode 100644 api/src/org/labkey/api/secrets/ExternalSecretProvider.java create mode 100644 api/src/org/labkey/api/secrets/SecretProperty.java create mode 100644 api/src/org/labkey/api/secrets/SecretService.java create mode 100644 api/src/org/labkey/api/util/LabKeyProcessBuilder.java create mode 100644 core/src/org/labkey/core/secrets/SecretServiceImpl.java diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 69dadae1fdd..2e11c36b188 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -2561,67 +2561,65 @@ public FileLike getStartupPropDirectory() private void loadStartupProps() { FileLike propsDir = getStartupPropDirectory(); - if (null == propsDir) - return; - - if (!propsDir.isDirectory()) - return; - FileLike newinstall = propsDir.resolveChild("newinstall"); - if (newinstall.isFile()) + if (null != propsDir && propsDir.isDirectory()) { - _log.debug("'newinstall' file detected: {}", newinstall.toNioPathForRead()); + FileLike newinstall = propsDir.resolveChild("newinstall"); + if (newinstall.isFile()) + { + _log.debug("'newinstall' file detected: {}", newinstall.toNioPathForRead()); - _newInstall = true; + _newInstall = true; - // propsDir is readonly, so we need to cheat to get a File - var newInstallFile = newinstall.toNioPathForRead().toFile(); - if (newInstallFile.canWrite()) - newInstallFile.delete(); + // propsDir is readonly, so we need to cheat to get a File + var newInstallFile = newinstall.toNioPathForRead().toFile(); + if (newInstallFile.canWrite()) + newInstallFile.delete(); + else + throw new ConfigurationException("file 'newinstall' exists, but is not writeable: " + newinstall.toNioPathForRead()); + } else - throw new ConfigurationException("file 'newinstall' exists, but is not writeable: " + newinstall.toNioPathForRead()); - } - else - { - _log.debug("no 'newinstall' file detected"); - } - - List propFiles = propsDir.getChildren().stream().filter(f -> f.getName().endsWith(".properties")).toList(); + { + _log.debug("no 'newinstall' file detected"); + } - if (!propFiles.isEmpty()) - { - List sortedPropFiles = propFiles.stream() - .sorted(Comparator.comparing(FileLike::getName).reversed()) - .toList(); + List propFiles = propsDir.getChildren().stream().filter(f -> f.getName().endsWith(".properties")).toList(); - for (FileLike propFile : sortedPropFiles) + if (!propFiles.isEmpty()) { - _log.debug("loading propsFile: {}", propFile.toNioPathForRead()); + List sortedPropFiles = propFiles.stream() + .sorted(Comparator.comparing(FileLike::getName).reversed()) + .toList(); - try (InputStream in = propFile.openInputStream()) + for (FileLike propFile : sortedPropFiles) { - Properties props = new Properties(); - props.load(in); + _log.debug("loading propsFile: {}", propFile.toNioPathForRead()); - for (Map.Entry entry : props.entrySet()) + try (InputStream in = propFile.openInputStream()) { - if (entry.getKey() instanceof String && entry.getValue() instanceof String) + Properties props = new Properties(); + props.load(in); + + for (Map.Entry entry : props.entrySet()) { - _log.trace("property '{}' resolved to value: '{}'", entry.getKey(), entry.getValue()); + if (entry.getKey() instanceof String && entry.getValue() instanceof String) + { + _log.trace("property '{}' resolved to value: '{}'", entry.getKey(), entry.getValue()); - addStartupPropertyEntry(entry.getKey().toString(), entry.getValue().toString()); + addStartupPropertyEntry(entry.getKey().toString(), entry.getValue().toString()); + } } } - } - catch (Exception e) - { - _log.error("Error parsing startup config properties file '{}'", propFile.toNioPathForRead(), e); + catch (Exception e) + { + _log.error("Error parsing startup config properties file '{}'", propFile.toNioPathForRead(), e); + } } } - } - else - { - _log.debug("no propFiles to load"); + else + { + _log.debug("no propFiles to load"); + } } // load any system properties with the labkey prop prefix diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index f31b0ff68d3..ed46777b4f5 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -25,6 +25,7 @@ import org.labkey.api.reader.Readers; import org.labkey.api.reports.report.r.ParamReplacementSvc; import org.labkey.api.util.ExceptionUtil; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.util.FileUtil; import org.labkey.api.util.QuietCloser; import org.labkey.api.util.URIUtil; @@ -126,7 +127,7 @@ public Object eval(String script, ScriptContext context) throws ScriptException protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptException { String[] params = formatCommand(scriptFile, context); - ProcessBuilder pb = new ProcessBuilder(params); + LabKeyProcessBuilder pb = new LabKeyProcessBuilder(params); pb = pb.directory(getWorkingDir(context).toNioPathForRead().toFile()); final long timeout = getTimeout(context); @@ -318,7 +319,7 @@ else if (value.startsWith("\"")) * Execute the external script engine in separate process * @return the exit code for the invocation - 0 if the process completed successfully. */ - protected int runProcess(ScriptContext context, ProcessBuilder pb, StringBuffer output, long timeout, TimeUnit timeoutUnit) + protected int runProcess(ScriptContext context, LabKeyProcessBuilder pb, StringBuffer output, long timeout, TimeUnit timeoutUnit) { Process proc; try diff --git a/api/src/org/labkey/api/secrets/ExternalSecretProvider.java b/api/src/org/labkey/api/secrets/ExternalSecretProvider.java new file mode 100644 index 00000000000..e03e1c630e3 --- /dev/null +++ b/api/src/org/labkey/api/secrets/ExternalSecretProvider.java @@ -0,0 +1,20 @@ +package org.labkey.api.secrets; + +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; + +/** + * SPI for external secret stores (e.g., AWS SSM Parameter Store). + * Implementations are registered with {@link SecretService} and consulted at higher + * priority than startup-property secrets. The provider is responsible for caching; + * {@link #refreshAll} is called periodically by a timer managed by {@link SecretService}. + */ +public interface ExternalSecretProvider +{ + /** Returns the secret for the given property name, or null if not available in this provider. */ + @Nullable String getSecret(String propertyName); + + /** Refresh cached values for all registered property names. Called on a timer. */ + void refreshAll(Collection propertyNames); +} diff --git a/api/src/org/labkey/api/secrets/SecretProperty.java b/api/src/org/labkey/api/secrets/SecretProperty.java new file mode 100644 index 00000000000..a6294ea0e5c --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretProperty.java @@ -0,0 +1,40 @@ +package org.labkey.api.secrets; + +import org.labkey.api.settings.StartupProperty; + +/** + * Describes a named secret that a module needs to access. Register instances with + * {@link SecretService#register} during module startup; retrieve values via + * {@link SecretService#getSecret}. + * + * Startup property file convention: {@code secret.=} + * Environment variable convention: {@code labkey.prop.secret.=} + */ +public class SecretProperty implements StartupProperty +{ + private final String _name; + private final String _description; + + public SecretProperty(String name) + { + this(name, "Secret: " + name); + } + + public SecretProperty(String name, String description) + { + _name = name; + _description = description; + } + + @Override + public String getPropertyName() + { + return _name; + } + + @Override + public String getDescription() + { + return _description; + } +} diff --git a/api/src/org/labkey/api/secrets/SecretService.java b/api/src/org/labkey/api/secrets/SecretService.java new file mode 100644 index 00000000000..652577febea --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretService.java @@ -0,0 +1,58 @@ +package org.labkey.api.secrets; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.services.ServiceRegistry; + +/** + * Internal service that provides access to secrets (API keys, passwords, etc.) without + * requiring callers to know where the secret is stored. Secrets may come from startup + * property files, process environment variables, or an external store such as AWS SSM. + * + *

Modules should: + *

    + *
  1. Declare each secret as a {@code static final SecretProperty} constant.
    2. + *
  2. Call {@link #register} in their {@code doStartup()} method.
    3. + *
  3. Call {@link #getSecret} wherever the value is needed.
    4. + *
+ * + *

Startup property file convention: {@code secret.=} + */ +public interface SecretService +{ + static @NotNull SecretService get() + { + SecretService svc = ServiceRegistry.get().getService(SecretService.class); + if (svc == null) + throw new IllegalStateException("SecretService has not been initialized"); + return svc; + } + + static void setInstance(SecretService service) + { + ServiceRegistry.get().registerService(SecretService.class, service); + } + + /** + * Declare that the calling module may request the named secret. Should be called + * from {@code Module.doStartup()}. Registration is for documentation and filtering + * (e.g., admin env-var page redaction); it does not affect whether a value is returned. + */ + void register(@NotNull SecretProperty property); + + /** + * Retrieve the value of a secret. Returns {@code null} if the secret has not been + * configured in any source. Never logs or caches the returned value. + */ + @Nullable String getSecret(@NotNull SecretProperty property); + + /** + * Register an {@link ExternalSecretProvider} (e.g., AWS SSM). The provider is + * consulted at higher priority than startup-property secrets. Should be called after + * server startup is complete so that startup properties are loaded first. + * + * TODO This is a total place holder. Secrets may need to be available very eary, so we + * TODO need to consider a typical registerProvider() interface will work well. + */ + void setExternalProvider(@NotNull ExternalSecretProvider provider); +} diff --git a/api/src/org/labkey/api/util/GUID.java b/api/src/org/labkey/api/util/GUID.java index c245e355944..914a391b31d 100644 --- a/api/src/org/labkey/api/util/GUID.java +++ b/api/src/org/labkey/api/util/GUID.java @@ -126,7 +126,7 @@ private static String networkIdentifier() try { - ProcessBuilder cmd = new ProcessBuilder("ipconfig.exe", "/all"); + LabKeyProcessBuilder cmd = new LabKeyProcessBuilder("ipconfig.exe", "/all"); cmd.redirectErrorStream(true); p = cmd.start(); } @@ -135,7 +135,7 @@ private static String networkIdentifier() { try { - ProcessBuilder cmd = new ProcessBuilder("ifconfig", "-a"); + LabKeyProcessBuilder cmd = new LabKeyProcessBuilder("ifconfig", "-a"); cmd.redirectErrorStream(true); p = cmd.start(); } diff --git a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java new file mode 100644 index 00000000000..cbba925b032 --- /dev/null +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -0,0 +1,96 @@ +package org.labkey.api.util; + +import org.labkey.api.secrets.SecretService; +import org.labkey.api.services.ServiceRegistry; + +import java.io.File; +import java.io.IOException; +import java.util.List; +import java.util.Map; + +/** + * Wrapper for {@link ProcessBuilder} that removes secret-named environment variables + * from the subprocess environment before the process starts. This prevents secrets from leaking + * to untrusted child processes. + * + *

Variables are removed (silently) if their name: + *

    + *
  • matches any property name registered with {@link SecretService}, or
    • + *
  • contains "secret" (case-insensitive), or
    • + *
  • contains "password" (case-insensitive).
    • + *
+ * + *

Use this class wherever {@link ProcessBuilder} would otherwise be used. A Checkstyle rule + * flags direct instantiation of {@code java.lang.ProcessBuilder} as a reminder. + */ +public class LabKeyProcessBuilder +{ + private final ProcessBuilder _pb; + + public LabKeyProcessBuilder(List command) + { + _pb = new ProcessBuilder(command); + sanitizeEnvironment(); + } + + public LabKeyProcessBuilder(String... command) + { + _pb = new ProcessBuilder(command); + sanitizeEnvironment(); + } + + public LabKeyProcessBuilder(File directory, List command) + { + _pb = new ProcessBuilder(command); + _pb.directory(directory); + sanitizeEnvironment(); + } + + public List command() + { + return _pb.command(); + } + + public Map environment() + { + return _pb.environment(); + } + + public File directory() + { + return _pb.directory(); + } + + public LabKeyProcessBuilder directory(File directory) + { + _pb.directory(directory); + return this; + } + + public LabKeyProcessBuilder redirectErrorStream(boolean redirectErrorStream) + { + _pb.redirectErrorStream(redirectErrorStream); + return this; + } + + public Process start() throws IOException + { + return _pb.start(); + } + + /** Returns the underlying {@link ProcessBuilder} for APIs that require it directly. */ + public ProcessBuilder processBuilder() + { + return _pb; + } + + private void sanitizeEnvironment() + { + SecretService secrets = ServiceRegistry.get().getService(SecretService.class); + + _pb.environment().keySet().removeIf(name -> + name.toLowerCase().contains("secret") || + name.toLowerCase().contains("password") + ); + } +} diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index a10324eec79..c7572cbe0e1 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -158,6 +158,7 @@ import org.labkey.api.settings.OptionalFeatureFlag; import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.settings.OptionalFeatureService.FeatureType; +import org.labkey.api.secrets.SecretService; import org.labkey.api.settings.ProductConfiguration; import org.labkey.api.settings.StandardStartupPropertyHandler; import org.labkey.api.settings.StartupPropertyEntry; @@ -295,6 +296,7 @@ import org.labkey.core.thumbnail.ThumbnailServiceImpl; import org.labkey.core.user.LimitActiveUsersSettings; import org.labkey.core.user.UserController; +import org.labkey.core.secrets.SecretServiceImpl; import org.labkey.core.vcs.VcsServiceImpl; import org.labkey.core.view.ShortURLServiceImpl; import org.labkey.core.view.TableViewFormTestCase; @@ -418,6 +420,11 @@ public boolean hasScripts() @Override protected void init() { + SecretServiceImpl secretService = new SecretServiceImpl(); + secretService.handleStartupProperties(); + SecretService.setInstance(secretService); + ContextListener.addShutdownListener(ShutdownListener.of("SecretService", null, secretService::shutdown)); + ContainerService.setInstance(new ContainerServiceImpl()); FolderSerializationRegistry.setInstance(new FolderSerializationRegistryImpl()); ExternalToolsViewService.setInstance(new ExternalToolsViewServiceImpl()); @@ -1495,6 +1502,7 @@ public TabDisplayMode getTabDisplayMode() OutOfRangeDisplayColumn.TestCase.class, PostgreSqlVersion.TestCase.class, ScriptEngineManagerImpl.TestCase.class, + SecretServiceImpl.TestCase.class, StatsServiceImpl.TestCase.class, diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index d0907560eed..bffe51b1e1a 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -3396,7 +3396,14 @@ public class EnvironmentVariablesAction extends SimpleViewAction @Override public ModelAndView getView(Object o, BindException errors) { - return new JspView<>("/org/labkey/core/admin/properties.jsp", System.getenv()); + Map env = new LinkedHashMap<>(System.getenv()); + env.replaceAll((name, value) -> { + String lc = name.toLowerCase(); + if (lc.contains("secret") || lc.contains("password")) + return "[REDACTED]"; + return value; + }); + return new JspView<>("/org/labkey/core/admin/properties.jsp", env); } @Override diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java new file mode 100644 index 00000000000..874efd3d658 --- /dev/null +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -0,0 +1,180 @@ +package org.labkey.core.secrets; + +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.secrets.ExternalSecretProvider; +import org.labkey.api.secrets.SecretProperty; +import org.labkey.api.secrets.SecretService; +import org.labkey.api.settings.LenientStartupPropertyHandler; +import org.labkey.api.settings.StartupPropertyEntry; +import org.labkey.api.util.logging.LogHelper; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +public class SecretServiceImpl implements SecretService +{ + private static final Logger LOG = LogHelper.getLogger(SecretServiceImpl.class, "Secret service"); + static final String STARTUP_PROPERTY_SCOPE = "secret"; + + // Secrets loaded from startup properties / environment variables + private final Map _startupSecrets = new ConcurrentHashMap<>(); + // Names of all registered SecretProperty instances (for documentation and env filtering) + private final Set _registeredNames = Collections.synchronizedSet(new HashSet<>()); + + private volatile ExternalSecretProvider _externalProvider = null; + + // Refresh timer — used when an ExternalSecretProvider is registered + private final ScheduledExecutorService _scheduler = Executors.newSingleThreadScheduledExecutor(r -> { + Thread t = new Thread(r, "SecretService-refresh"); + t.setDaemon(true); + return t; + }); + private volatile ScheduledFuture _refreshFuture = null; + + /** + * Register a LenientStartupPropertyHandler to capture all "secret.*" entries from + * startup property files and their corresponding environment variables. Call this + * from CoreModule.init() after constructing and registering the service. + */ + public void handleStartupProperties() + { + ModuleLoader.getInstance().handleStartupProperties( + new LenientStartupPropertyHandler<>(STARTUP_PROPERTY_SCOPE, _SecretDocProperty.INSTANCE) + { + @Override + public void handle(Collection entries) + { + entries.forEach(entry -> _startupSecrets.put(entry.getName(), entry.getValue())); + } + }); + } + + @Override + public void register(@NotNull SecretProperty property) + { + _registeredNames.add(property.getPropertyName()); + } + + @Override + public @Nullable String getSecret(@NotNull SecretProperty property) + { + String name = property.getPropertyName(); + + // External provider (e.g., SSM) has highest priority + ExternalSecretProvider provider = _externalProvider; + if (provider != null) + { + String value = provider.getSecret(name); + if (value != null) + { + LOG.debug("Secret '{}' resolved from external provider", name); + return value; + } + } + + String value = _startupSecrets.get(name); + if (value != null) + LOG.debug("Secret '{}' resolved from startup properties", name); + return value; + } + + @Override + public void setExternalProvider(@NotNull ExternalSecretProvider provider) + { + _externalProvider = provider; + scheduleRefresh(); + } + + /** Cancel the refresh timer on server shutdown. */ + public void shutdown() + { + if (_refreshFuture != null) + _refreshFuture.cancel(false); + _scheduler.shutdown(); + } + + private void scheduleRefresh() + { + if (_refreshFuture != null) + _refreshFuture.cancel(false); + + _refreshFuture = _scheduler.scheduleAtFixedRate(() -> { + ExternalSecretProvider p = _externalProvider; + if (p != null) + p.refreshAll(Collections.unmodifiableSet(_registeredNames)); + }, 5, 5, TimeUnit.MINUTES); + } + + // Singleton SecretProperty used as the documentation entry for the "secret" scope on the + // Startup Properties admin page. The scope name is "secret" and the property name is "*" + // to indicate that any property name is accepted under this scope. + private static class _SecretDocProperty extends SecretProperty + { + static final _SecretDocProperty INSTANCE = new _SecretDocProperty(); + + private _SecretDocProperty() + { + super(STARTUP_PROPERTY_SCOPE, "Any secret registered via SecretService.register(). " + + "Provide as: secret.="); + } + } + + public static class TestCase extends Assert + { + @Test + public void testStartupPropertyLoading() + { + SecretServiceImpl svc = new SecretServiceImpl(); + svc._startupSecrets.put("test.API_KEY", "abc123"); + + SecretProperty prop = new SecretProperty("test.API_KEY", "Test API Key"); + svc.register(prop); + + assertEquals("abc123", svc.getSecret(prop)); + } + + @Test + public void testMissingSecret() + { + SecretServiceImpl svc = new SecretServiceImpl(); + SecretProperty prop = new SecretProperty("nonexistent.KEY"); + assertNull(svc.getSecret(prop)); + } + + @Test + public void testExternalProviderPriority() + { + SecretServiceImpl svc = new SecretServiceImpl(); + svc._startupSecrets.put("my.KEY", "from-startup"); + + svc.setExternalProvider(new ExternalSecretProvider() + { + @Override + public @Nullable String getSecret(String propertyName) + { + return "my.KEY".equals(propertyName) ? "from-external" : null; + } + + @Override + public void refreshAll(Collection propertyNames) {} + }); + + SecretProperty prop = new SecretProperty("my.KEY"); + assertEquals("from-external", svc.getSecret(prop)); + svc.shutdown(); + } + } +} diff --git a/core/src/org/labkey/core/vcs/VcsServiceImpl.java b/core/src/org/labkey/core/vcs/VcsServiceImpl.java index 287ffebc112..d0b774cc75f 100644 --- a/core/src/org/labkey/core/vcs/VcsServiceImpl.java +++ b/core/src/org/labkey/core/vcs/VcsServiceImpl.java @@ -4,6 +4,7 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.vcs.Vcs; import org.labkey.api.vcs.VcsService; @@ -103,7 +104,7 @@ private void execute(String... command) { String cl = log(_rootDirectory, command); - ProcessBuilder builder = new ProcessBuilder(command); + LabKeyProcessBuilder builder = new LabKeyProcessBuilder(command); builder.directory(_rootDirectory); try { diff --git a/pipeline/src/org/labkey/pipeline/analysis/CommandTaskImpl.java b/pipeline/src/org/labkey/pipeline/analysis/CommandTaskImpl.java index 864c736bb1b..460727f4254 100644 --- a/pipeline/src/org/labkey/pipeline/analysis/CommandTaskImpl.java +++ b/pipeline/src/org/labkey/pipeline/analysis/CommandTaskImpl.java @@ -48,6 +48,7 @@ import org.labkey.api.security.SecurityManager.TransformSession; import org.labkey.api.settings.AppProps; import org.labkey.api.util.FileType; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.util.NetworkDrive; import org.labkey.api.util.Path; import org.labkey.vfs.FileLike; @@ -690,7 +691,7 @@ protected boolean runCommand(RecordedAction action, @Nullable String apiKey, @Nu { Map replacements = container == null ? Collections.emptyMap() : createReplacements(null, apiKey, container); - ProcessBuilder pb = new ProcessBuilder(_factory.toArgs(this, replacements)); + LabKeyProcessBuilder pb = new LabKeyProcessBuilder(_factory.toArgs(this, replacements)); applyEnvironment(pb); List args = pb.command(); @@ -732,7 +733,7 @@ protected boolean runCommand(RecordedAction action, @Nullable String apiKey, @Nu action.setStartTime(new Date()); action.addParameter(RecordedAction.COMMAND_LINE_PARAM, commandLine); int timeout = _factory._timeout != null ? _factory._timeout : 0; - getJob().runSubProcess(pb, _wd.getDir(), fileOutput, lineInterval, false, timeout, TimeUnit.SECONDS); + getJob().runSubProcess(pb.processBuilder(), _wd.getDir(), fileOutput, lineInterval, false, timeout, TimeUnit.SECONDS); action.setEndTime(new Date()); return true; } @@ -777,7 +778,7 @@ public void testSubstitution() } } - private void applyEnvironment(ProcessBuilder pb) + private void applyEnvironment(LabKeyProcessBuilder pb) { Map originalEnvironment = new HashMap<>(pb.environment()); for (Map.Entry entry : _factory._environment.entrySet()) diff --git a/pipeline/src/org/labkey/pipeline/cluster/ClusterStartup.java b/pipeline/src/org/labkey/pipeline/cluster/ClusterStartup.java index 3fc7dafa737..d6a1df84bf9 100644 --- a/pipeline/src/org/labkey/pipeline/cluster/ClusterStartup.java +++ b/pipeline/src/org/labkey/pipeline/cluster/ClusterStartup.java @@ -29,6 +29,7 @@ import org.labkey.api.reader.Readers; import org.labkey.api.test.TestWhen; import org.labkey.api.util.ContextListener; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.util.FileUtil; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.PageFlowUtil; @@ -225,7 +226,7 @@ public void testBadPath() throws IOException, InterruptedException protected String executeJobRemote(List args, int expectedExitCode) throws IOException, InterruptedException { - ProcessBuilder pb = new ProcessBuilder(args); + LabKeyProcessBuilder pb = new LabKeyProcessBuilder(args); pb.directory(_tempDir); pb.redirectErrorStream(true); Process proc = pb.start(); From e49b0861e64bcc45445278e4f12046ce3888acb2 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 15 May 2026 09:56:52 -0700 Subject: [PATCH 2/9] comment out the time stuff for now environment variable implementation startup ordering --- core/src/org/labkey/core/CoreModule.java | 2 +- .../core/secrets/SecretServiceImpl.java | 63 ++++++++++--------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index c7572cbe0e1..540caec21fb 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -421,7 +421,6 @@ public boolean hasScripts() protected void init() { SecretServiceImpl secretService = new SecretServiceImpl(); - secretService.handleStartupProperties(); SecretService.setInstance(secretService); ContextListener.addShutdownListener(ShutdownListener.of("SecretService", null, secretService::shutdown)); @@ -973,6 +972,7 @@ public void startupAfterSpringConfig(ModuleContext moduleContext) checkForMissingDbViews(); + ((SecretServiceImpl)SecretService.get()).handleStartupProperties(); ProductConfiguration.handleStartupProperties(); // This listener deletes all properties; make sure it executes after most of the other listeners diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index 874efd3d658..798a428201a 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -1,5 +1,6 @@ package org.labkey.core.secrets; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -14,10 +15,7 @@ import org.labkey.api.util.logging.LogHelper; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -31,18 +29,18 @@ public class SecretServiceImpl implements SecretService // Secrets loaded from startup properties / environment variables private final Map _startupSecrets = new ConcurrentHashMap<>(); - // Names of all registered SecretProperty instances (for documentation and env filtering) - private final Set _registeredNames = Collections.synchronizedSet(new HashSet<>()); + // Registered SecretProperty instances keyed by property name (for documentation and env filtering) + private final Map _registeredSecrets = new ConcurrentHashMap<>(); private volatile ExternalSecretProvider _externalProvider = null; - // Refresh timer — used when an ExternalSecretProvider is registered - private final ScheduledExecutorService _scheduler = Executors.newSingleThreadScheduledExecutor(r -> { - Thread t = new Thread(r, "SecretService-refresh"); - t.setDaemon(true); - return t; - }); - private volatile ScheduledFuture _refreshFuture = null; +// // Refresh timer — used when an ExternalSecretProvider is registered +// private final ScheduledExecutorService _scheduler = Executors.newSingleThreadScheduledExecutor(r -> { +// Thread t = new Thread(r, "SecretService-refresh"); +// t.setDaemon(true); +// return t; +// }); +// private volatile ScheduledFuture _refreshFuture = null; /** * Register a LenientStartupPropertyHandler to capture all "secret.*" entries from @@ -58,6 +56,12 @@ public void handleStartupProperties() public void handle(Collection entries) { entries.forEach(entry -> _startupSecrets.put(entry.getName(), entry.getValue())); + for (var name : _registeredSecrets.keySet()) + { + var s = System.getenv(name); + if (StringUtils.isNotBlank(s)) + _startupSecrets.put(name,s); + } } }); } @@ -65,13 +69,16 @@ public void handle(Collection entries) @Override public void register(@NotNull SecretProperty property) { - _registeredNames.add(property.getPropertyName()); + _registeredSecrets.put(property.getPropertyName(), property); } @Override public @Nullable String getSecret(@NotNull SecretProperty property) { String name = property.getPropertyName(); + var registered = _registeredSecrets.get(name); + if (registered != property) + return null; // External provider (e.g., SSM) has highest priority ExternalSecretProvider provider = _externalProvider; @@ -95,28 +102,28 @@ public void register(@NotNull SecretProperty property) public void setExternalProvider(@NotNull ExternalSecretProvider provider) { _externalProvider = provider; - scheduleRefresh(); +// scheduleRefresh(); } /** Cancel the refresh timer on server shutdown. */ public void shutdown() { - if (_refreshFuture != null) - _refreshFuture.cancel(false); - _scheduler.shutdown(); +// if (_refreshFuture != null) +// _refreshFuture.cancel(false); +// _scheduler.shutdown(); } - private void scheduleRefresh() - { - if (_refreshFuture != null) - _refreshFuture.cancel(false); - - _refreshFuture = _scheduler.scheduleAtFixedRate(() -> { - ExternalSecretProvider p = _externalProvider; - if (p != null) - p.refreshAll(Collections.unmodifiableSet(_registeredNames)); - }, 5, 5, TimeUnit.MINUTES); - } +// private void scheduleRefresh() +// { +// if (_refreshFuture != null) +// _refreshFuture.cancel(false); +// +// _refreshFuture = _scheduler.scheduleAtFixedRate(() -> { +// ExternalSecretProvider p = _externalProvider; +// if (p != null) +// p.refreshAll(_registeredSecrets.keySet()); +// }, 5, 5, TimeUnit.MINUTES); +// } // Singleton SecretProperty used as the documentation entry for the "secret" scope on the // Startup Properties admin page. The scope name is "secret" and the property name is "*" From d612904201ac33fd76ff72d192db13e03834c445 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 15 May 2026 10:02:45 -0700 Subject: [PATCH 3/9] duplicate test --- core/src/org/labkey/core/secrets/SecretServiceImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index 798a428201a..b4cbb61bb12 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -12,6 +12,7 @@ import org.labkey.api.secrets.SecretService; import org.labkey.api.settings.LenientStartupPropertyHandler; import org.labkey.api.settings.StartupPropertyEntry; +import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.logging.LogHelper; import java.util.Collection; @@ -69,7 +70,9 @@ public void handle(Collection entries) @Override public void register(@NotNull SecretProperty property) { - _registeredSecrets.put(property.getPropertyName(), property); + var prev = _registeredSecrets.put(property.getPropertyName(), property); + if (null != prev) + throw new ConfigurationException("Duplicate secret registered: " + property.getPropertyName()); } @Override From 1dd9bf219ebf62dc6c439ea7e2d59c9a8694589b Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Fri, 15 May 2026 13:15:42 -0700 Subject: [PATCH 4/9] fix mcp init --- .../labkey/api/secrets/SecretProperty.java | 3 ++- .../org/labkey/api/secrets/SecretService.java | 9 ++++++++ .../labkey/api/util/LabKeyProcessBuilder.java | 10 ++++----- .../labkey/core/admin/AdminController.java | 4 +++- .../core/secrets/SecretServiceImpl.java | 21 ++++++++++++++++++- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/secrets/SecretProperty.java b/api/src/org/labkey/api/secrets/SecretProperty.java index a6294ea0e5c..7d87136f4c4 100644 --- a/api/src/org/labkey/api/secrets/SecretProperty.java +++ b/api/src/org/labkey/api/secrets/SecretProperty.java @@ -8,7 +8,8 @@ * {@link SecretService#getSecret}. * * Startup property file convention: {@code secret.=} - * Environment variable convention: {@code labkey.prop.secret.=} + * Java property convention: {@code -Plabkey.prop.secret.=} + * Environment variable convention: {@code export =} */ public class SecretProperty implements StartupProperty { diff --git a/api/src/org/labkey/api/secrets/SecretService.java b/api/src/org/labkey/api/secrets/SecretService.java index 652577febea..a3c062e4796 100644 --- a/api/src/org/labkey/api/secrets/SecretService.java +++ b/api/src/org/labkey/api/secrets/SecretService.java @@ -43,9 +43,18 @@ static void setInstance(SecretService service) /** * Retrieve the value of a secret. Returns {@code null} if the secret has not been * configured in any source. Never logs or caches the returned value. + * + *

Identity contract: the {@code property} argument must be the exact + * {@code static final} instance that was passed to {@link #register}. A freshly constructed + * {@code new SecretProperty("SOME_KEY")} will always return {@code null}, even if a secret + * with that name is configured. This prevents unregistered callers from reading secrets + * they did not declare. */ @Nullable String getSecret(@NotNull SecretProperty property); + /** Returns true if the given property name has been registered via {@link #register}. */ + boolean isRegisteredSecret(@NotNull String name); + /** * Register an {@link ExternalSecretProvider} (e.g., AWS SSM). The provider is * consulted at higher priority than startup-property secrets. Should be called after diff --git a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java index cbba925b032..6a01703318c 100644 --- a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -87,10 +87,10 @@ public ProcessBuilder processBuilder() private void sanitizeEnvironment() { SecretService secrets = ServiceRegistry.get().getService(SecretService.class); - - _pb.environment().keySet().removeIf(name -> - name.toLowerCase().contains("secret") || - name.toLowerCase().contains("password") - ); + _pb.environment().keySet().removeIf(name -> { + String lc = name.toLowerCase(); + return lc.contains("secret") || lc.contains("password") || + (secrets != null && secrets.isRegisteredSecret(name)); + }); } } diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index bffe51b1e1a..464852dddf0 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -188,6 +188,7 @@ import org.labkey.api.reports.ExternalScriptEngineDefinition; import org.labkey.api.reports.LabKeyScriptEngineManager; import org.labkey.api.search.SearchService; +import org.labkey.api.secrets.SecretService; import org.labkey.api.security.ActionNames; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; @@ -3396,10 +3397,11 @@ public class EnvironmentVariablesAction extends SimpleViewAction @Override public ModelAndView getView(Object o, BindException errors) { + SecretService secrets = SecretService.get(); Map env = new LinkedHashMap<>(System.getenv()); env.replaceAll((name, value) -> { String lc = name.toLowerCase(); - if (lc.contains("secret") || lc.contains("password")) + if (lc.contains("secret") || lc.contains("password") || secrets.isRegisteredSecret(name)) return "[REDACTED]"; return value; }); diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index b4cbb61bb12..b64ec722993 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -101,6 +101,12 @@ public void register(@NotNull SecretProperty property) return value; } + @Override + public boolean isRegisteredSecret(@NotNull String name) + { + return _registeredSecrets.containsKey(name); + } + @Override public void setExternalProvider(@NotNull ExternalSecretProvider provider) { @@ -157,10 +163,22 @@ public void testStartupPropertyLoading() } @Test - public void testMissingSecret() + public void testUnregisteredSecretReturnsNull() + { + // getSecret() requires the same instance passed to register(); a fresh instance with + // the same name must not be able to retrieve a secret it didn't declare. + SecretServiceImpl svc = new SecretServiceImpl(); + SecretProperty prop = new SecretProperty("some.KEY"); + svc._startupSecrets.put("some.KEY", "value"); + assertNull("unregistered property must not retrieve a secret", svc.getSecret(prop)); + } + + @Test + public void testRegisteredSecretWithNoValueReturnsNull() { SecretServiceImpl svc = new SecretServiceImpl(); SecretProperty prop = new SecretProperty("nonexistent.KEY"); + svc.register(prop); assertNull(svc.getSecret(prop)); } @@ -183,6 +201,7 @@ public void refreshAll(Collection propertyNames) {} }); SecretProperty prop = new SecretProperty("my.KEY"); + svc.register(prop); assertEquals("from-external", svc.getSecret(prop)); svc.shutdown(); } From 6c57b128cdf24a6e7566f6b290721ba0189bbbc1 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Thu, 21 May 2026 09:01:35 -0700 Subject: [PATCH 5/9] Support SSM-backed properties in application.properties and secrets --- .../api/secrets/ExternalSecretProvider.java | 20 --- .../labkey/api/secrets/SecretProvider.java | 18 +++ .../org/labkey/api/secrets/SecretService.java | 25 +++- .../org/labkey/api/secrets/SecretStatus.java | 19 +++ core/src/org/labkey/core/CoreModule.java | 1 + .../labkey/core/admin/AdminController.java | 48 +++++++ .../EnvironmentVariableSecretProvider.java | 22 +++ .../core/secrets/SecretServiceImpl.java | 134 ++++++++++-------- .../secrets/SsmExternalSecretProvider.java | 74 ++++++++++ .../StartupPropertySecretProvider.java | 37 +++++ 10 files changed, 309 insertions(+), 89 deletions(-) delete mode 100644 api/src/org/labkey/api/secrets/ExternalSecretProvider.java create mode 100644 api/src/org/labkey/api/secrets/SecretProvider.java create mode 100644 api/src/org/labkey/api/secrets/SecretStatus.java create mode 100644 core/src/org/labkey/core/secrets/EnvironmentVariableSecretProvider.java create mode 100644 core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java create mode 100644 core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java diff --git a/api/src/org/labkey/api/secrets/ExternalSecretProvider.java b/api/src/org/labkey/api/secrets/ExternalSecretProvider.java deleted file mode 100644 index e03e1c630e3..00000000000 --- a/api/src/org/labkey/api/secrets/ExternalSecretProvider.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.labkey.api.secrets; - -import org.jetbrains.annotations.Nullable; - -import java.util.Collection; - -/** - * SPI for external secret stores (e.g., AWS SSM Parameter Store). - * Implementations are registered with {@link SecretService} and consulted at higher - * priority than startup-property secrets. The provider is responsible for caching; - * {@link #refreshAll} is called periodically by a timer managed by {@link SecretService}. - */ -public interface ExternalSecretProvider -{ - /** Returns the secret for the given property name, or null if not available in this provider. */ - @Nullable String getSecret(String propertyName); - - /** Refresh cached values for all registered property names. Called on a timer. */ - void refreshAll(Collection propertyNames); -} diff --git a/api/src/org/labkey/api/secrets/SecretProvider.java b/api/src/org/labkey/api/secrets/SecretProvider.java new file mode 100644 index 00000000000..625936feb10 --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretProvider.java @@ -0,0 +1,18 @@ +package org.labkey.api.secrets; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * SPI for a secret source. Implementations cover built-in sources (startup property files, + * environment variables) and external stores (e.g., AWS SSM Parameter Store). + * Providers are consulted in priority order by {@link SecretService}. + */ +public interface SecretProvider +{ + /** Returns the secret for the given property name, or null if not available from this source. */ + @Nullable String getSecret(String propertyName); + + /** Human-readable name for this source, shown on the admin secrets page. */ + @NotNull String getDescription(); +} diff --git a/api/src/org/labkey/api/secrets/SecretService.java b/api/src/org/labkey/api/secrets/SecretService.java index a3c062e4796..959147781a8 100644 --- a/api/src/org/labkey/api/secrets/SecretService.java +++ b/api/src/org/labkey/api/secrets/SecretService.java @@ -4,6 +4,8 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.services.ServiceRegistry; +import java.util.List; + /** * Internal service that provides access to secrets (API keys, passwords, etc.) without * requiring callers to know where the secret is stored. Secrets may come from startup @@ -56,12 +58,21 @@ static void setInstance(SecretService service) boolean isRegisteredSecret(@NotNull String name); /** - * Register an {@link ExternalSecretProvider} (e.g., AWS SSM). The provider is - * consulted at higher priority than startup-property secrets. Should be called after - * server startup is complete so that startup properties are loaded first. - * - * TODO This is a total place holder. Secrets may need to be available very eary, so we - * TODO need to consider a typical registerProvider() interface will work well. + * Register a high-priority {@link SecretProvider} (e.g., AWS SSM). This provider is + * consulted before the built-in startup-property and environment-variable providers. + */ + void setExternalProvider(@NotNull SecretProvider provider); + + /** + * Returns read-only status for every registered secret, sorted by name. + * Never includes secret values — safe to display in admin UI. + */ + @NotNull List getSecretStatuses(); + + /** + * Returns a human-readable description of the active external provider (e.g., + * "AWS SSM Parameter Store"), or {@code null} if no external provider is registered. + * The external provider takes priority over startup-property and environment-variable sources. */ - void setExternalProvider(@NotNull ExternalSecretProvider provider); + @Nullable String getExternalProviderDescription(); } diff --git a/api/src/org/labkey/api/secrets/SecretStatus.java b/api/src/org/labkey/api/secrets/SecretStatus.java new file mode 100644 index 00000000000..a877d9f29bb --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretStatus.java @@ -0,0 +1,19 @@ +package org.labkey.api.secrets; + +import org.jetbrains.annotations.Nullable; + +/** + * Read-only status of a registered secret — suitable for admin UI display. + * Never contains the secret value itself. + * + * @param source description of the provider that holds this secret + * (e.g. "Startup property file", "Environment variable"), or + * {@code null} if no provider has a value for it + */ +public record SecretStatus(String name, String description, @Nullable String source) +{ + public boolean isSet() + { + return source != null; + } +} diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 540caec21fb..b6cf8154f87 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -297,6 +297,7 @@ import org.labkey.core.user.LimitActiveUsersSettings; import org.labkey.core.user.UserController; import org.labkey.core.secrets.SecretServiceImpl; + import org.labkey.core.vcs.VcsServiceImpl; import org.labkey.core.view.ShortURLServiceImpl; import org.labkey.core.view.TableViewFormTestCase; diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 401b07ab437..d3a4350df87 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -35,6 +35,7 @@ import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; +import org.apache.commons.lang3.mutable.MutableInt; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -189,6 +190,7 @@ import org.labkey.api.reports.LabKeyScriptEngineManager; import org.labkey.api.search.SearchService; import org.labkey.api.secrets.SecretService; +import org.labkey.api.secrets.SecretStatus; import org.labkey.api.security.ActionNames; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; @@ -426,8 +428,10 @@ import static org.labkey.api.util.DOM.P; import static org.labkey.api.util.DOM.SPAN; import static org.labkey.api.util.DOM.STYLE; +import static org.labkey.api.util.DOM.STRONG; import static org.labkey.api.util.DOM.TABLE; import static org.labkey.api.util.DOM.TD; +import static org.labkey.api.util.DOM.TH; import static org.labkey.api.util.DOM.TR; import static org.labkey.api.util.DOM.UL; import static org.labkey.api.util.DOM.at; @@ -506,6 +510,7 @@ public static void registerAdminConsoleLinks() AdminConsole.addLink(Diagnostics, "queries", getQueriesURL(null)); AdminConsole.addLink(Diagnostics, "reset site errors", new ActionURL(ResetErrorMarkAction.class, root), AdminPermission.class); AdminConsole.addLink(Diagnostics, "running threads", new ActionURL(ShowThreadsAction.class, root)); + AdminConsole.addLink(Diagnostics, "secrets", new ActionURL(SecretsAction.class, root), SiteAdminPermission.class); AdminConsole.addLink(Diagnostics, "site validation", new ActionURL(ConfigureSiteValidationAction.class, root), AdminPermission.class); AdminConsole.addLink(Diagnostics, "sql scripts", new ActionURL(SqlScriptController.ScriptsAction.class, root), AdminOperationsPermission.class); AdminConsole.addLink(Diagnostics, "suspicious activity", new ActionURL(SuspiciousAction.class, root)); @@ -3476,6 +3481,48 @@ public void addNavTrail(NavTree root) } } + @RequiresSiteAdmin + public class SecretsAction extends SimpleViewAction + { + @Override + public ModelAndView getView(Object o, BindException errors) + { + List statuses = SecretService.get().getSecretStatuses(); + + List rows = new ArrayList<>(); + + if (statuses.isEmpty()) + { + return new HtmlView(DIV("No secrets have been registered with SecretService.")); + } + + MutableInt rowIndex = new MutableInt(0); + + Renderable table = DOM.TABLE(cl("table-condensed labkey-data-region table-bordered").at(style, "border:none"), + TR( + TH(at(style, "border:none; width:220px;"), STRONG("Secret Name")), + TH(at(style, "border:none; width:300px;"), STRONG("Description")), + TH(at(style, "border:none;"), STRONG("Source")) + ), + statuses.stream().map(status -> + TR( + cl(rowIndex.getAndIncrement() % 2 == 0 ? "labkey-alternate-row" : "labkey-row"), + TD(status.name()), + TD(status.description()), + status.isSet() + ? TD(status.source()) + : TD(SPAN(cl("labkey-error"), "Not configured")) + ))); + return new HtmlView(table); + } + + @Override + public void addNavTrail(NavTree root) + { + addAdminNavTrail(root, "Secrets", this.getClass()); + } + } + public static class ConfigureSystemMaintenanceForm { @@ -12469,6 +12516,7 @@ controller.new ShowThreadsAction(), // @RequiresSiteAdmin assertForRequiresSiteAdmin(user, controller.new EnvironmentVariablesAction(), + controller.new SecretsAction(), controller.new SystemMaintenanceAction(), controller.new SystemPropertiesAction(), new GetPendingRequestCountAction(), diff --git a/core/src/org/labkey/core/secrets/EnvironmentVariableSecretProvider.java b/core/src/org/labkey/core/secrets/EnvironmentVariableSecretProvider.java new file mode 100644 index 00000000000..4fd57806c20 --- /dev/null +++ b/core/src/org/labkey/core/secrets/EnvironmentVariableSecretProvider.java @@ -0,0 +1,22 @@ +package org.labkey.core.secrets; + +import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.secrets.SecretProvider; + +class EnvironmentVariableSecretProvider implements SecretProvider +{ + @Override + public @Nullable String getSecret(String propertyName) + { + String value = System.getenv(propertyName); + return StringUtils.isNotBlank(value) ? value : null; + } + + @Override + public @NotNull String getDescription() + { + return "Environment variable"; + } +} diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index b64ec722993..a26901310c8 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -1,52 +1,54 @@ package org.labkey.core.secrets; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; import org.labkey.api.module.ModuleLoader; -import org.labkey.api.secrets.ExternalSecretProvider; import org.labkey.api.secrets.SecretProperty; +import org.labkey.api.secrets.SecretProvider; import org.labkey.api.secrets.SecretService; +import org.labkey.api.secrets.SecretStatus; import org.labkey.api.settings.LenientStartupPropertyHandler; import org.labkey.api.settings.StartupPropertyEntry; import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.logging.LogHelper; import java.util.Collection; +import java.util.Comparator; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; public class SecretServiceImpl implements SecretService { private static final Logger LOG = LogHelper.getLogger(SecretServiceImpl.class, "Secret service"); static final String STARTUP_PROPERTY_SCOPE = "secret"; - // Secrets loaded from startup properties / environment variables - private final Map _startupSecrets = new ConcurrentHashMap<>(); - // Registered SecretProperty instances keyed by property name (for documentation and env filtering) + // Registered SecretProperty instances keyed by property name private final Map _registeredSecrets = new ConcurrentHashMap<>(); - private volatile ExternalSecretProvider _externalProvider = null; + // Providers consulted in priority order: external (highest) → env vars → startup properties + private volatile SecretProvider _externalProvider = null; + private final SecretProvider _envProvider = new EnvironmentVariableSecretProvider(); + private final StartupPropertySecretProvider _startupProvider; -// // Refresh timer — used when an ExternalSecretProvider is registered -// private final ScheduledExecutorService _scheduler = Executors.newSingleThreadScheduledExecutor(r -> { -// Thread t = new Thread(r, "SecretService-refresh"); -// t.setDaemon(true); -// return t; -// }); -// private volatile ScheduledFuture _refreshFuture = null; + public SecretServiceImpl() + { + this(new StartupPropertySecretProvider()); + } + + private SecretServiceImpl(StartupPropertySecretProvider startupProvider) + { + _startupProvider = startupProvider; + } /** * Register a LenientStartupPropertyHandler to capture all "secret.*" entries from - * startup property files and their corresponding environment variables. Call this - * from CoreModule.init() after constructing and registering the service. + * startup property files. Call this from CoreModule.init() after constructing and + * registering the service. */ public void handleStartupProperties() { @@ -56,13 +58,7 @@ public void handleStartupProperties() @Override public void handle(Collection entries) { - entries.forEach(entry -> _startupSecrets.put(entry.getName(), entry.getValue())); - for (var name : _registeredSecrets.keySet()) - { - var s = System.getenv(name); - if (StringUtils.isNotBlank(s)) - _startupSecrets.put(name,s); - } + _startupProvider.load(entries); } }); } @@ -83,22 +79,16 @@ public void register(@NotNull SecretProperty property) if (registered != property) return null; - // External provider (e.g., SSM) has highest priority - ExternalSecretProvider provider = _externalProvider; - if (provider != null) + for (SecretProvider provider : activeProviders()) { String value = provider.getSecret(name); if (value != null) { - LOG.debug("Secret '{}' resolved from external provider", name); + LOG.debug("Secret '{}' resolved from {}", name, provider.getDescription()); return value; } } - - String value = _startupSecrets.get(name); - if (value != null) - LOG.debug("Secret '{}' resolved from startup properties", name); - return value; + return null; } @Override @@ -108,31 +98,45 @@ public boolean isRegisteredSecret(@NotNull String name) } @Override - public void setExternalProvider(@NotNull ExternalSecretProvider provider) + public void setExternalProvider(@NotNull SecretProvider provider) { _externalProvider = provider; -// scheduleRefresh(); } - /** Cancel the refresh timer on server shutdown. */ - public void shutdown() + @Override + public @NotNull List getSecretStatuses() + { + return _registeredSecrets.values().stream() + .filter(s -> !Objects.isNull(s.getPropertyName())) + .sorted(Comparator.comparing(SecretProperty::getPropertyName)) + .map(prop -> { + String name = prop.getPropertyName(); + String source = activeProviders().stream() + .filter(p -> p.getSecret(name) != null) + .map(SecretProvider::getDescription) + .findFirst() + .orElse(null); + return new SecretStatus(name, prop.getDescription(), source); + }) + .toList(); + } + + @Override + public @Nullable String getExternalProviderDescription() { -// if (_refreshFuture != null) -// _refreshFuture.cancel(false); -// _scheduler.shutdown(); + SecretProvider provider = _externalProvider; + return provider != null ? provider.getDescription() : null; } -// private void scheduleRefresh() -// { -// if (_refreshFuture != null) -// _refreshFuture.cancel(false); -// -// _refreshFuture = _scheduler.scheduleAtFixedRate(() -> { -// ExternalSecretProvider p = _externalProvider; -// if (p != null) -// p.refreshAll(_registeredSecrets.keySet()); -// }, 5, 5, TimeUnit.MINUTES); -// } + public void shutdown() {} + + private List activeProviders() + { + SecretProvider external = _externalProvider; + return external != null + ? List.of(external, _envProvider, _startupProvider) + : List.of(_envProvider, _startupProvider); + } // Singleton SecretProperty used as the documentation entry for the "secret" scope on the // Startup Properties admin page. The scope name is "secret" and the property name is "*" @@ -153,8 +157,7 @@ public static class TestCase extends Assert @Test public void testStartupPropertyLoading() { - SecretServiceImpl svc = new SecretServiceImpl(); - svc._startupSecrets.put("test.API_KEY", "abc123"); + SecretServiceImpl svc = new SecretServiceImpl(startupProviderWith("test.API_KEY", "abc123")); SecretProperty prop = new SecretProperty("test.API_KEY", "Test API Key"); svc.register(prop); @@ -167,9 +170,8 @@ public void testUnregisteredSecretReturnsNull() { // getSecret() requires the same instance passed to register(); a fresh instance with // the same name must not be able to retrieve a secret it didn't declare. - SecretServiceImpl svc = new SecretServiceImpl(); + SecretServiceImpl svc = new SecretServiceImpl(startupProviderWith("some.KEY", "value")); SecretProperty prop = new SecretProperty("some.KEY"); - svc._startupSecrets.put("some.KEY", "value"); assertNull("unregistered property must not retrieve a secret", svc.getSecret(prop)); } @@ -185,10 +187,9 @@ public void testRegisteredSecretWithNoValueReturnsNull() @Test public void testExternalProviderPriority() { - SecretServiceImpl svc = new SecretServiceImpl(); - svc._startupSecrets.put("my.KEY", "from-startup"); + SecretServiceImpl svc = new SecretServiceImpl(startupProviderWith("my.KEY", "from-startup")); - svc.setExternalProvider(new ExternalSecretProvider() + svc.setExternalProvider(new SecretProvider() { @Override public @Nullable String getSecret(String propertyName) @@ -197,13 +198,22 @@ public void testExternalProviderPriority() } @Override - public void refreshAll(Collection propertyNames) {} + public @NotNull String getDescription() + { + return "Test provider"; + } }); SecretProperty prop = new SecretProperty("my.KEY"); svc.register(prop); assertEquals("from-external", svc.getSecret(prop)); - svc.shutdown(); + } + + private StartupPropertySecretProvider startupProviderWith(String name, String value) + { + StartupPropertySecretProvider provider = new StartupPropertySecretProvider(); + provider.loadDirect(name, value); + return provider; } } } diff --git a/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java b/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java new file mode 100644 index 00000000000..4217db30cd1 --- /dev/null +++ b/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java @@ -0,0 +1,74 @@ +package org.labkey.core.secrets; + +import org.jetbrains.annotations.Nullable; +import org.labkey.api.secrets.ExternalSecretProvider; + +import java.lang.reflect.Method; +import java.util.Collection; + +/** + * ExternalSecretProvider that delegates to SsmSecretBridge in the Spring Boot (parent) + * classloader. This allows the webapp to look up runtime secrets from AWS SSM Parameter + * Store at runtime without a direct dependency on the embedded module or the AWS SDK. + * + *

The bridge class ({@code org.labkey.embedded.SsmSecretBridge}) lives in the parent + * classloader and is found by reflection. When running outside embedded mode (standalone + * Tomcat) or when SSM is not configured, {@link #createIfAvailable()} returns null and + * no external provider is registered. + * + *

SSM parameter names are constructed by the bridge as {@code {prefix}{propertyName}}, + * where the prefix defaults to {@code /labkey/} and is configurable via + * {@code context.awsParameterStore.prefix} in {@code application.properties}. + */ +public class SsmExternalSecretProvider implements ExternalSecretProvider +{ + private static final String BRIDGE_CLASS = "org.labkey.embedded.SsmSecretBridge"; + + private final Method _getSecretMethod; + + private SsmExternalSecretProvider(Method getSecretMethod) + { + _getSecretMethod = getSecretMethod; + } + + /** + * Returns a new instance if the SsmSecretBridge class is reachable via the parent + * classloader, or null when not running in embedded mode. + */ + public static @Nullable SsmExternalSecretProvider createIfAvailable() + { + try + { + ClassLoader parent = Thread.currentThread().getContextClassLoader().getParent(); + if (parent == null) + return null; + Class bridgeClass = parent.loadClass(BRIDGE_CLASS); + Method method = bridgeClass.getMethod("getSecret", String.class); + return new SsmExternalSecretProvider(method); + } + catch (ClassNotFoundException | NoSuchMethodException e) + { + return null; + } + } + + @Override + public @Nullable String getSecret(String propertyName) + { + try + { + return (String) _getSecretMethod.invoke(null, propertyName); + } + catch (ReflectiveOperationException e) + { + return null; + } + } + + @Override + public void refreshAll(Collection propertyNames) + { + // SSM is called on each getSecret() call; caching and refresh can be added later + // when SecretServiceImpl's refresh timer is enabled + } +} diff --git a/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java b/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java new file mode 100644 index 00000000000..2fd248df20d --- /dev/null +++ b/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java @@ -0,0 +1,37 @@ +package org.labkey.core.secrets; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.secrets.SecretProvider; +import org.labkey.api.settings.StartupPropertyEntry; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +class StartupPropertySecretProvider implements SecretProvider +{ + private final Map _secrets = new HashMap<>(); + + void load(Collection entries) + { + entries.forEach(entry -> _secrets.put(entry.getName(), entry.getValue())); + } + + void loadDirect(String name, String value) + { + _secrets.put(name, value); + } + + @Override + public @Nullable String getSecret(String propertyName) + { + return _secrets.get(propertyName); + } + + @Override + public @NotNull String getDescription() + { + return "Startup property file"; + } +} From 2de6ddbf964b8404d5f4affa3785d1292a48524a Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Thu, 21 May 2026 11:37:23 -0700 Subject: [PATCH 6/9] Assorted fixes --- .../core/secrets/SecretServiceImpl.java | 3 +- .../secrets/SsmExternalSecretProvider.java | 74 ------------------- 2 files changed, 1 insertion(+), 76 deletions(-) delete mode 100644 core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index a26901310c8..7ea986228b2 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -19,7 +19,6 @@ import java.util.Comparator; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; public class SecretServiceImpl implements SecretService @@ -107,7 +106,7 @@ public void setExternalProvider(@NotNull SecretProvider provider) public @NotNull List getSecretStatuses() { return _registeredSecrets.values().stream() - .filter(s -> !Objects.isNull(s.getPropertyName())) + .filter(s -> s.getPropertyName() != null) .sorted(Comparator.comparing(SecretProperty::getPropertyName)) .map(prop -> { String name = prop.getPropertyName(); diff --git a/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java b/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java deleted file mode 100644 index 4217db30cd1..00000000000 --- a/core/src/org/labkey/core/secrets/SsmExternalSecretProvider.java +++ /dev/null @@ -1,74 +0,0 @@ -package org.labkey.core.secrets; - -import org.jetbrains.annotations.Nullable; -import org.labkey.api.secrets.ExternalSecretProvider; - -import java.lang.reflect.Method; -import java.util.Collection; - -/** - * ExternalSecretProvider that delegates to SsmSecretBridge in the Spring Boot (parent) - * classloader. This allows the webapp to look up runtime secrets from AWS SSM Parameter - * Store at runtime without a direct dependency on the embedded module or the AWS SDK. - * - *

The bridge class ({@code org.labkey.embedded.SsmSecretBridge}) lives in the parent - * classloader and is found by reflection. When running outside embedded mode (standalone - * Tomcat) or when SSM is not configured, {@link #createIfAvailable()} returns null and - * no external provider is registered. - * - *

SSM parameter names are constructed by the bridge as {@code {prefix}{propertyName}}, - * where the prefix defaults to {@code /labkey/} and is configurable via - * {@code context.awsParameterStore.prefix} in {@code application.properties}. - */ -public class SsmExternalSecretProvider implements ExternalSecretProvider -{ - private static final String BRIDGE_CLASS = "org.labkey.embedded.SsmSecretBridge"; - - private final Method _getSecretMethod; - - private SsmExternalSecretProvider(Method getSecretMethod) - { - _getSecretMethod = getSecretMethod; - } - - /** - * Returns a new instance if the SsmSecretBridge class is reachable via the parent - * classloader, or null when not running in embedded mode. - */ - public static @Nullable SsmExternalSecretProvider createIfAvailable() - { - try - { - ClassLoader parent = Thread.currentThread().getContextClassLoader().getParent(); - if (parent == null) - return null; - Class bridgeClass = parent.loadClass(BRIDGE_CLASS); - Method method = bridgeClass.getMethod("getSecret", String.class); - return new SsmExternalSecretProvider(method); - } - catch (ClassNotFoundException | NoSuchMethodException e) - { - return null; - } - } - - @Override - public @Nullable String getSecret(String propertyName) - { - try - { - return (String) _getSecretMethod.invoke(null, propertyName); - } - catch (ReflectiveOperationException e) - { - return null; - } - } - - @Override - public void refreshAll(Collection propertyNames) - { - // SSM is called on each getSecret() call; caching and refresh can be added later - // when SecretServiceImpl's refresh timer is enabled - } -} From be259671f50de9df9af24403615e6383e752b4ba Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 22 May 2026 11:23:47 -0700 Subject: [PATCH 7/9] Misc improvements --- .../labkey/api/util/LabKeyProcessBuilder.java | 7 +++++-- .../labkey/core/secrets/SecretServiceImpl.java | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java index 6a01703318c..da638925d1e 100644 --- a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -20,8 +20,8 @@ *

  • contains "password" (case-insensitive).
    • * * - *

    Use this class wherever {@link ProcessBuilder} would otherwise be used. A Checkstyle rule - * flags direct instantiation of {@code java.lang.ProcessBuilder} as a reminder. + *

    Use this class wherever {@link ProcessBuilder} would otherwise be used. An IntelliJ inspection + * (SSBasedInspection) flags direct instantiation of {@code java.lang.ProcessBuilder} as a reminder. */ public class LabKeyProcessBuilder { @@ -29,18 +29,21 @@ public class LabKeyProcessBuilder public LabKeyProcessBuilder(List command) { + //noinspection SSBasedInspection _pb = new ProcessBuilder(command); sanitizeEnvironment(); } public LabKeyProcessBuilder(String... command) { + //noinspection SSBasedInspection _pb = new ProcessBuilder(command); sanitizeEnvironment(); } public LabKeyProcessBuilder(File directory, List command) { + //noinspection SSBasedInspection _pb = new ProcessBuilder(command); _pb.directory(directory); sanitizeEnvironment(); diff --git a/core/src/org/labkey/core/secrets/SecretServiceImpl.java b/core/src/org/labkey/core/secrets/SecretServiceImpl.java index 7ea986228b2..34df4035b3f 100644 --- a/core/src/org/labkey/core/secrets/SecretServiceImpl.java +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -127,7 +127,21 @@ public void setExternalProvider(@NotNull SecretProvider provider) return provider != null ? provider.getDescription() : null; } - public void shutdown() {} + public void shutdown() + { + SecretProvider external = _externalProvider; + if (external instanceof AutoCloseable closeable) + { + try + { + closeable.close(); + } + catch (Exception e) + { + LOG.warn("Error closing external secret provider", e); + } + } + } private List activeProviders() { From 2631e3724566abf482f510305b0759c53936b77c Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 22 May 2026 16:05:26 -0700 Subject: [PATCH 8/9] Consolidate secret redaction logic; revert MCP embedding/model change --- .../org/labkey/api/util/LabKeyProcessBuilder.java | 14 +++++++++----- .../src/org/labkey/core/admin/AdminController.java | 9 ++------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java index da638925d1e..21b4ee92c24 100644 --- a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -88,12 +88,16 @@ public ProcessBuilder processBuilder() } private void sanitizeEnvironment() + { + _pb.environment().keySet().removeIf(LabKeyProcessBuilder::isSecret); + } + + /** @return true if the property name is known to be or inferred to be a secret (credential, etc) */ + public static boolean isSecret(String propertyName) { SecretService secrets = ServiceRegistry.get().getService(SecretService.class); - _pb.environment().keySet().removeIf(name -> { - String lc = name.toLowerCase(); - return lc.contains("secret") || lc.contains("password") || - (secrets != null && secrets.isRegisteredSecret(name)); - }); + String lc = propertyName.toLowerCase(); + return lc.contains("secret") || lc.contains("password") || lc.contains("apikey") || + (secrets != null && secrets.isRegisteredSecret(propertyName)); } } diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index d3a4350df87..2d6e9743c79 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -268,6 +268,7 @@ import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.HttpsUtil; import org.labkey.api.util.JsonUtil; +import org.labkey.api.util.LabKeyProcessBuilder; import org.labkey.api.util.LinkBuilder; import org.labkey.api.util.MailHelper; import org.labkey.api.util.MemTracker; @@ -3447,14 +3448,8 @@ public class EnvironmentVariablesAction extends SimpleViewAction @Override public ModelAndView getView(Object o, BindException errors) { - SecretService secrets = SecretService.get(); Map env = new LinkedHashMap<>(System.getenv()); - env.replaceAll((name, value) -> { - String lc = name.toLowerCase(); - if (lc.contains("secret") || lc.contains("password") || secrets.isRegisteredSecret(name)) - return "[REDACTED]"; - return value; - }); + env.replaceAll((name, value) -> LabKeyProcessBuilder.isSecret(name) ? "[REDACTED]" : value); return new JspView<>("/org/labkey/core/admin/properties.jsp", env); } From 6ed0b80db0ff0290013fd7fb3ef7d7924f724ba0 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 22 May 2026 17:16:25 -0700 Subject: [PATCH 9/9] Minor tweaks --- api/src/org/labkey/api/util/LabKeyProcessBuilder.java | 2 +- .../labkey/core/secrets/StartupPropertySecretProvider.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java index 21b4ee92c24..e27a2c92110 100644 --- a/api/src/org/labkey/api/util/LabKeyProcessBuilder.java +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -97,7 +97,7 @@ public static boolean isSecret(String propertyName) { SecretService secrets = ServiceRegistry.get().getService(SecretService.class); String lc = propertyName.toLowerCase(); - return lc.contains("secret") || lc.contains("password") || lc.contains("apikey") || + return lc.contains("secret") || lc.contains("password") || lc.contains("apikey") || lc.contains("_key") || lc.contains("token") || (secrets != null && secrets.isRegisteredSecret(propertyName)); } } diff --git a/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java b/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java index 2fd248df20d..a6dc0b7677d 100644 --- a/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java +++ b/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java @@ -2,16 +2,16 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.collections.CopyOnWriteHashMap; import org.labkey.api.secrets.SecretProvider; import org.labkey.api.settings.StartupPropertyEntry; import java.util.Collection; -import java.util.HashMap; import java.util.Map; class StartupPropertySecretProvider implements SecretProvider { - private final Map _secrets = new HashMap<>(); + private final Map _secrets = new CopyOnWriteHashMap<>(); void load(Collection entries) {