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/SecretProperty.java b/api/src/org/labkey/api/secrets/SecretProperty.java new file mode 100644 index 00000000000..7d87136f4c4 --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretProperty.java @@ -0,0 +1,41 @@ +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.=} + * Java property convention: {@code -Plabkey.prop.secret.=} + * Environment variable convention: {@code export =} + */ +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/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 new file mode 100644 index 00000000000..959147781a8 --- /dev/null +++ b/api/src/org/labkey/api/secrets/SecretService.java @@ -0,0 +1,78 @@ +package org.labkey.api.secrets; + +import org.jetbrains.annotations.NotNull; +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 + * 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. + * + *

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 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. + */ + @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/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..e27a2c92110 --- /dev/null +++ b/api/src/org/labkey/api/util/LabKeyProcessBuilder.java @@ -0,0 +1,103 @@ +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. An IntelliJ inspection + * (SSBasedInspection) flags direct instantiation of {@code java.lang.ProcessBuilder} as a reminder. + */ +public class LabKeyProcessBuilder +{ + private final ProcessBuilder _pb; + + 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(); + } + + 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() + { + _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); + String lc = propertyName.toLowerCase(); + 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/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index a10324eec79..b6cf8154f87 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,8 @@ 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 +421,10 @@ public boolean hasScripts() @Override protected void init() { + SecretServiceImpl secretService = new SecretServiceImpl(); + SecretService.setInstance(secretService); + ContextListener.addShutdownListener(ShutdownListener.of("SecretService", null, secretService::shutdown)); + ContainerService.setInstance(new ContainerServiceImpl()); FolderSerializationRegistry.setInstance(new FolderSerializationRegistryImpl()); ExternalToolsViewService.setInstance(new ExternalToolsViewServiceImpl()); @@ -966,6 +973,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 @@ -1495,6 +1503,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 15d0ba871c1..2d6e9743c79 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; @@ -188,6 +189,8 @@ 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.secrets.SecretStatus; import org.labkey.api.security.ActionNames; import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; @@ -265,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; @@ -425,8 +429,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; @@ -505,6 +511,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)); @@ -3441,7 +3448,9 @@ 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) -> LabKeyProcessBuilder.isSecret(name) ? "[REDACTED]" : value); + return new JspView<>("/org/labkey/core/admin/properties.jsp", env); } @Override @@ -3467,6 +3476,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 { @@ -12460,6 +12511,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 new file mode 100644 index 00000000000..34df4035b3f --- /dev/null +++ b/core/src/org/labkey/core/secrets/SecretServiceImpl.java @@ -0,0 +1,232 @@ +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.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.concurrent.ConcurrentHashMap; + +public class SecretServiceImpl implements SecretService +{ + private static final Logger LOG = LogHelper.getLogger(SecretServiceImpl.class, "Secret service"); + static final String STARTUP_PROPERTY_SCOPE = "secret"; + + // Registered SecretProperty instances keyed by property name + private final Map _registeredSecrets = new ConcurrentHashMap<>(); + + // 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; + + public SecretServiceImpl() + { + this(new StartupPropertySecretProvider()); + } + + private SecretServiceImpl(StartupPropertySecretProvider startupProvider) + { + _startupProvider = startupProvider; + } + + /** + * Register a LenientStartupPropertyHandler to capture all "secret.*" entries from + * startup property files. 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) + { + _startupProvider.load(entries); + } + }); + } + + @Override + public void register(@NotNull SecretProperty property) + { + var prev = _registeredSecrets.put(property.getPropertyName(), property); + if (null != prev) + throw new ConfigurationException("Duplicate secret registered: " + property.getPropertyName()); + } + + @Override + public @Nullable String getSecret(@NotNull SecretProperty property) + { + String name = property.getPropertyName(); + var registered = _registeredSecrets.get(name); + if (registered != property) + return null; + + for (SecretProvider provider : activeProviders()) + { + String value = provider.getSecret(name); + if (value != null) + { + LOG.debug("Secret '{}' resolved from {}", name, provider.getDescription()); + return value; + } + } + return null; + } + + @Override + public boolean isRegisteredSecret(@NotNull String name) + { + return _registeredSecrets.containsKey(name); + } + + @Override + public void setExternalProvider(@NotNull SecretProvider provider) + { + _externalProvider = provider; + } + + @Override + public @NotNull List getSecretStatuses() + { + return _registeredSecrets.values().stream() + .filter(s -> s.getPropertyName() != null) + .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() + { + SecretProvider provider = _externalProvider; + return provider != null ? provider.getDescription() : null; + } + + 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() + { + 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 "*" + // 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(startupProviderWith("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 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(startupProviderWith("some.KEY", "value")); + SecretProperty prop = new SecretProperty("some.KEY"); + 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)); + } + + @Test + public void testExternalProviderPriority() + { + SecretServiceImpl svc = new SecretServiceImpl(startupProviderWith("my.KEY", "from-startup")); + + svc.setExternalProvider(new SecretProvider() + { + @Override + public @Nullable String getSecret(String propertyName) + { + return "my.KEY".equals(propertyName) ? "from-external" : null; + } + + @Override + public @NotNull String getDescription() + { + return "Test provider"; + } + }); + + SecretProperty prop = new SecretProperty("my.KEY"); + svc.register(prop); + assertEquals("from-external", svc.getSecret(prop)); + } + + 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/StartupPropertySecretProvider.java b/core/src/org/labkey/core/secrets/StartupPropertySecretProvider.java new file mode 100644 index 00000000000..a6dc0b7677d --- /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.collections.CopyOnWriteHashMap; +import org.labkey.api.secrets.SecretProvider; +import org.labkey.api.settings.StartupPropertyEntry; + +import java.util.Collection; +import java.util.Map; + +class StartupPropertySecretProvider implements SecretProvider +{ + private final Map _secrets = new CopyOnWriteHashMap<>(); + + 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"; + } +} 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();