mirror of
https://github.com/iflytek/skillhub.git
synced 2026-11-14 09:01:16 +00:00
fix(authoring): enforce deployment environment authorization
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
4aab1ec48d
commit
ccc8006408
13 changed files with 567 additions and 17 deletions
|
|
@ -116,6 +116,7 @@ services:
|
|||
SKILLHUB_SECURITY_SCANNER_RETRY_MAX: ${SKILLHUB_SECURITY_SCANNER_RETRY_MAX:-3}
|
||||
SKILLHUB_SCAN_STREAM_RECLAIM_MIN_IDLE: ${SKILLHUB_SCAN_STREAM_RECLAIM_MIN_IDLE:-PT16M}
|
||||
SKILLHUB_BUILTIN_SKILLS_ENABLED: ${SKILLHUB_BUILTIN_SKILLS_ENABLED:-true}
|
||||
SKILLHUB_AUTHORING_SCRIPT_ENVIRONMENT_ALLOWLIST: ${SKILLHUB_AUTHORING_SCRIPT_ENVIRONMENT_ALLOWLIST:-}
|
||||
SKILLHUB_AUTH_DIRECT_ENABLED: ${SKILLHUB_AUTH_DIRECT_ENABLED:-false}
|
||||
SKILLHUB_TRACING_MODE: ${SKILLHUB_TRACING_MODE:-none}
|
||||
SKILLHUB_LOG_FORMAT: ${SKILLHUB_LOG_FORMAT:-json}
|
||||
|
|
|
|||
|
|
@ -27,3 +27,32 @@
|
|||
数据库迁移为未发布的 `V69__authoring_platform.sql`。正 revision、非负大小/计数及状态转换在业务代码中验证;数据库保留主键、外键和唯一性结构约束。头像/邀请 PR #947 使用 V70–V74,须在 V69 合入后验证其升级顺序,避免把较低编号的迁移留到已应用较高版本之后。
|
||||
|
||||
API 的完整字段、操作与响应以服务生成的 OpenAPI 为准;本文件不替代接口定义。
|
||||
|
||||
## 本地脚本的环境变量授权
|
||||
|
||||
部署管理员通过 `skillhub.authoring.local-script.environment-allowlist` 设置可选变量名称的上限;
|
||||
环境覆盖项为 `SKILLHUB_AUTHORING_SCRIPT_ENVIRONMENT_ALLOWLIST`,多个名称用英文逗号分隔。
|
||||
缺省为空:除适配器固定的启动环境外,草稿不能选择额外的服务器变量。例如部署可以显式允许
|
||||
`AUTHORING_DEMO_SETTING`,草稿绑定的 `envAllowlist` 只能选择该名称或更小的子集,不能自行授予权限。
|
||||
名称必须是匹配 `[A-Z][A-Z0-9_]{0,63}` 的大写字符串,区分大小写;绑定不保存变量值。
|
||||
已获准但在服务器进程中未定义的变量不转发。
|
||||
|
||||
此配置向所有能够运行本地脚本的草稿拥有者授予权限,包括普通成员;它不是按用户或命名空间
|
||||
划分的凭据授权。只允许可以向这些用户披露的值,因为脚本及运行日志均可能暴露收到的值。
|
||||
不要为了运行草稿而将服务器凭据加入此集合;此类需求应另行设计有范围限制的凭据绑定。
|
||||
|
||||
绑定保存、每轮 CONFIG 验证和每次任务执行前均使用同一策略检查。请求中只要有一个未授权名称,
|
||||
整组请求就在读取宿主变量之前被拒绝,不静默丢弃非法名称。任务执行入口还会在后端探测、
|
||||
执行计划和进程创建之前复验;CONFIG 层记录拒绝后可能仍检查后端可用性,但不会因此获得变量权限。
|
||||
已有或排队的绑定不能保留旧权限,执行时仍须通过当前已生效配置。
|
||||
更改部署配置后必须重启应用才能新增或撤销权限;没有自动刷新磁盘配置的机制。
|
||||
已运行进程中的值无法召回,轮换或撤销已转发的值之前应按现有取消流程停止相应运行。
|
||||
|
||||
`PATH`、`HOME`、`LANG` 及 Windows 启动名称 `SYSTEMROOT`、`WINDIR`、`COMSPEC`、
|
||||
`PATHEXT`、`TEMP`、`TMP` 均为保留项,部署集合不能包含它们,草稿也不能覆盖适配器的可信启动环境。
|
||||
授权规则同时适用于 INLINE 和 DOCKER;默认执行模式仍为 Docker。
|
||||
Docker CLI 的控制环境继续继承以保持守护进程连接配置,只有通过授权的已清理环境映射会以
|
||||
`-e` 形式传给脚本容器。容器隔离和输出脱敏均不能替代环境授权。
|
||||
|
||||
`compose.release.yml` 已为空默认透传此设置;其他自定义 Compose/Helm 部署需显式将同名变量
|
||||
传入服务器进程,或使用应用配置覆盖。仅在外层 shell 或未透传的 env 文件中设置不代表应用已生效。
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
package com.iflytek.skillhub.config;
|
||||
|
||||
import com.iflytek.skillhub.domain.authoring.service.ScriptEnvironmentPolicy;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
|
||||
/** Wires deployment configuration into the domain policy without a reverse module dependency. */
|
||||
@Configuration
|
||||
public class AuthoringEnvironmentConfig {
|
||||
|
||||
@Bean
|
||||
public ScriptEnvironmentPolicy scriptEnvironmentPolicy(AuthoringProperties properties) {
|
||||
return new ScriptEnvironmentPolicy(() -> properties.getLocalScript().getEnvironmentAllowlist());
|
||||
}
|
||||
}
|
||||
|
|
@ -68,6 +68,17 @@ public class AuthoringProperties {
|
|||
/** Where scripts run: INLINE on the server host (dev), DOCKER in a locked-down container (production). */
|
||||
private ScriptExecutionMode executionMode = ScriptExecutionMode.INLINE;
|
||||
|
||||
/** Deployment-owned variable names available to all local-script draft owners. Empty denies all. */
|
||||
private List<String> environmentAllowlist = List.of();
|
||||
|
||||
public List<String> getEnvironmentAllowlist() {
|
||||
return environmentAllowlist;
|
||||
}
|
||||
|
||||
public void setEnvironmentAllowlist(List<String> environmentAllowlist) {
|
||||
this.environmentAllowlist = environmentAllowlist == null ? List.of() : List.copyOf(environmentAllowlist);
|
||||
}
|
||||
|
||||
private final Docker docker = new Docker();
|
||||
|
||||
public boolean isEnabled() {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.service.authoring.adapter;
|
||||
|
||||
import com.iflytek.skillhub.config.AuthoringProperties;
|
||||
import com.iflytek.skillhub.domain.authoring.service.ScriptEnvironmentPolicy;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.RuntimeEventSink;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.RuntimeExecutionContext;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.SkillRuntimeAdapter;
|
||||
|
|
@ -24,7 +25,7 @@ import org.springframework.stereotype.Component;
|
|||
/**
|
||||
* Behavior-layer adapter that executes the skill's own scripts in an isolated working
|
||||
* directory with a scrubbed environment. Only whitelisted interpreters may run and only
|
||||
* environment variables named in the binding's {@code envAllowlist} pass through.
|
||||
* deployment-authorized variables selected by the binding's {@code envAllowlist} pass through.
|
||||
*
|
||||
* <p>Where the process actually runs is delegated to a {@link ScriptCommandBuilder}:
|
||||
* the inline backend spawns the interpreter on the server host, the docker backend
|
||||
|
|
@ -38,11 +39,24 @@ public class LocalScriptRuntimeAdapter implements SkillRuntimeAdapter {
|
|||
private static final long POLL_SLICE_MS = 200;
|
||||
|
||||
private final AuthoringProperties properties;
|
||||
private final ScriptEnvironmentPolicy environmentPolicy;
|
||||
private final Function<String, String> environmentLookup;
|
||||
private final Map<String, ScriptCommandBuilder> commandBuilders;
|
||||
|
||||
@org.springframework.beans.factory.annotation.Autowired
|
||||
public LocalScriptRuntimeAdapter(AuthoringProperties properties,
|
||||
List<ScriptCommandBuilder> commandBuilders) {
|
||||
List<ScriptCommandBuilder> commandBuilders,
|
||||
ScriptEnvironmentPolicy environmentPolicy) {
|
||||
this(properties, commandBuilders, environmentPolicy, System::getenv);
|
||||
}
|
||||
|
||||
LocalScriptRuntimeAdapter(AuthoringProperties properties,
|
||||
List<ScriptCommandBuilder> commandBuilders,
|
||||
ScriptEnvironmentPolicy environmentPolicy,
|
||||
Function<String, String> environmentLookup) {
|
||||
this.properties = properties;
|
||||
this.environmentPolicy = environmentPolicy;
|
||||
this.environmentLookup = environmentLookup;
|
||||
this.commandBuilders = commandBuilders.stream()
|
||||
.collect(Collectors.toUnmodifiableMap(ScriptCommandBuilder::backend,
|
||||
Function.identity()));
|
||||
|
|
@ -66,6 +80,8 @@ public class LocalScriptRuntimeAdapter implements SkillRuntimeAdapter {
|
|||
@Override
|
||||
public TaskResult execute(RuntimeExecutionContext context, ValidationTaskSpec task,
|
||||
RuntimeEventSink sink) throws Exception {
|
||||
// Recheck the effective policy before probing a backend or creating a process.
|
||||
Map<String, String> scrubbedEnvironment = scrubEnvironment(context);
|
||||
ScriptCommandBuilder builder = commandBuilder();
|
||||
if (!builder.available()) {
|
||||
throw new IllegalStateException(
|
||||
|
|
@ -75,8 +91,6 @@ public class LocalScriptRuntimeAdapter implements SkillRuntimeAdapter {
|
|||
String interpreter = resolveInterpreter(context);
|
||||
Path scriptPath = resolveInsideWorkingDirectory(context.workingDirectory(), task.script());
|
||||
String runTag = context.runId() + "-" + task.name();
|
||||
Map<String, String> scrubbedEnvironment = scrubEnvironment(context);
|
||||
|
||||
ScriptCommandBuilder.ScriptExecutionPlan plan = builder.plan(
|
||||
context.workingDirectory(), interpreter, scriptPath,
|
||||
task.safeArgs(), scrubbedEnvironment, runTag);
|
||||
|
|
@ -132,20 +146,20 @@ public class LocalScriptRuntimeAdapter implements SkillRuntimeAdapter {
|
|||
return "sh";
|
||||
}
|
||||
|
||||
private Map<String, String> scrubEnvironment(RuntimeExecutionContext context) {
|
||||
Map<String, String> scrubEnvironment(RuntimeExecutionContext context) {
|
||||
// Authorize the entire selection before looking up any host value. An old persisted
|
||||
// binding does not retain permission after the effective deployment policy changes.
|
||||
java.util.Set<String> selected = environmentPolicy.authorize(
|
||||
context.safeAdapterConfig().get("envAllowlist"));
|
||||
Map<String, String> environment = new java.util.LinkedHashMap<>();
|
||||
environment.put("PATH", System.getenv().getOrDefault("PATH", "/usr/bin:/bin"));
|
||||
String path = environmentLookup.apply("PATH");
|
||||
environment.put("PATH", path == null ? "/usr/bin:/bin" : path);
|
||||
environment.put("HOME", context.workingDirectory().toString());
|
||||
environment.put("LANG", "C.UTF-8");
|
||||
Object allowlist = context.safeAdapterConfig().get("envAllowlist");
|
||||
if (allowlist instanceof List<?> entries) {
|
||||
for (Object entry : entries) {
|
||||
if (entry != null) {
|
||||
String value = System.getenv(entry.toString());
|
||||
if (value != null) {
|
||||
environment.put(entry.toString(), value);
|
||||
}
|
||||
}
|
||||
for (String name : selected) {
|
||||
String value = environmentLookup.apply(name);
|
||||
if (value != null) {
|
||||
environment.put(name, value);
|
||||
}
|
||||
}
|
||||
return environment;
|
||||
|
|
|
|||
|
|
@ -145,6 +145,8 @@ skillhub:
|
|||
maintenance-interval-ms: ${SKILLHUB_AUTHORING_MAINTENANCE_INTERVAL_MS:30000}
|
||||
local-script:
|
||||
enabled: ${SKILLHUB_AUTHORING_LOCAL_SCRIPT_ENABLED:true}
|
||||
# Deployment ceiling; draft envAllowlist may only select a subset. Empty denies all.
|
||||
environment-allowlist: ${SKILLHUB_AUTHORING_SCRIPT_ENVIRONMENT_ALLOWLIST:}
|
||||
# docker is the production default: scripts run in a locked-down container —
|
||||
# no network, memory/cpu/pids caps, read-only rootfs, all capabilities dropped.
|
||||
# inline runs subprocesses on the server host and is only allowed with a
|
||||
|
|
|
|||
|
|
@ -0,0 +1,63 @@
|
|||
package com.iflytek.skillhub.config;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.boot.context.properties.bind.Bindable;
|
||||
import org.springframework.boot.context.properties.bind.Binder;
|
||||
import org.springframework.boot.context.properties.source.MapConfigurationPropertySource;
|
||||
import org.springframework.boot.context.properties.EnableConfigurationProperties;
|
||||
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.core.env.MapPropertySource;
|
||||
import com.iflytek.skillhub.domain.authoring.service.RuntimeBindingValidator;
|
||||
import com.iflytek.skillhub.service.authoring.adapter.LocalScriptRuntimeAdapter;
|
||||
import com.iflytek.skillhub.service.authoring.adapter.InlineScriptCommandBuilder;
|
||||
import static org.assertj.core.api.Assertions.*;
|
||||
|
||||
class AuthoringEnvironmentConfigTest {
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
@EnableConfigurationProperties(AuthoringProperties.class)
|
||||
static class PropertiesConfiguration { }
|
||||
|
||||
@Test void springContainerInjectsTheSharedPolicyIntoBothEntryPoints() {
|
||||
try (var context = new AnnotationConfigApplicationContext()) {
|
||||
context.getEnvironment().getPropertySources().addFirst(new MapPropertySource("dummy-deployment", Map.of(
|
||||
"skillhub.authoring.local-script.environment-allowlist", "AUTHORING_DEMO")));
|
||||
context.register(PropertiesConfiguration.class, AuthoringEnvironmentConfig.class,
|
||||
RuntimeBindingValidator.class, LocalScriptRuntimeAdapter.class, InlineScriptCommandBuilder.class);
|
||||
context.refresh();
|
||||
var validator = context.getBean(RuntimeBindingValidator.class);
|
||||
assertThat(context.getBean(LocalScriptRuntimeAdapter.class).agentType()).isEqualTo("local-script");
|
||||
var config = Map.<String, Object>of("envAllowlist", List.of("AUTHORING_DEMO"));
|
||||
assertThat(validator.validate("local-script", config, null, null)).isEmpty();
|
||||
context.getBean(AuthoringProperties.class).getLocalScript().setEnvironmentAllowlist(List.of());
|
||||
assertThat(validator.validate("local-script", config, null, null)).hasSize(1);
|
||||
}
|
||||
}
|
||||
@Test void defaultPropertiesDenyAllAndBeanSharesEffectiveSettings() {
|
||||
var properties = new AuthoringProperties();
|
||||
var policy = new AuthoringEnvironmentConfig().scriptEnvironmentPolicy(properties);
|
||||
assertThatThrownBy(() -> policy.authorize(List.of("AUTHORING_DEMO"))).isInstanceOf(IllegalArgumentException.class);
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of("AUTHORING_DEMO"));
|
||||
assertThat(policy.authorize(List.of("AUTHORING_DEMO"))).containsExactly("AUTHORING_DEMO");
|
||||
properties.getLocalScript().setEnvironmentAllowlist(null);
|
||||
assertThatThrownBy(() -> policy.authorize(List.of("AUTHORING_DEMO"))).isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
@Test void commaSeparatedSettingBindsWithoutChangingDockerMode() {
|
||||
var source = new MapConfigurationPropertySource(Map.of(
|
||||
"skillhub.authoring.local-script.environment-allowlist", "AUTHORING_DEMO,AUTHORING_OTHER",
|
||||
"skillhub.authoring.local-script.execution-mode", "docker"));
|
||||
var properties = new Binder(source).bind("skillhub.authoring", Bindable.of(AuthoringProperties.class)).get();
|
||||
assertThat(properties.getLocalScript().getExecutionMode()).isEqualTo(AuthoringProperties.ScriptExecutionMode.DOCKER);
|
||||
assertThat(properties.getLocalScript().getEnvironmentAllowlist()).containsExactly("AUTHORING_DEMO", "AUTHORING_OTHER");
|
||||
assertThat(new AuthoringEnvironmentConfig().scriptEnvironmentPolicy(properties).authorize(List.of("AUTHORING_DEMO")))
|
||||
.containsExactly("AUTHORING_DEMO");
|
||||
}
|
||||
@Test void explicitEmptySettingBindsToDenyAll() {
|
||||
var source = new MapConfigurationPropertySource(Map.of(
|
||||
"skillhub.authoring.local-script.environment-allowlist", ""));
|
||||
var properties = new Binder(source).bind("skillhub.authoring", Bindable.of(AuthoringProperties.class)).get();
|
||||
assertThat(properties.getLocalScript().getEnvironmentAllowlist()).isEmpty();
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,112 @@
|
|||
package com.iflytek.skillhub.service.authoring.adapter;
|
||||
|
||||
import com.iflytek.skillhub.config.AuthoringProperties;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.RuntimeExecutionContext;
|
||||
import com.iflytek.skillhub.domain.authoring.service.ScriptEnvironmentPolicy;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
import static org.assertj.core.api.Assertions.*;
|
||||
|
||||
class LocalScriptEnvironmentAuthorizationTest {
|
||||
@TempDir Path workspace;
|
||||
private static final String DEMO = "AUTHORING_DEMO";
|
||||
private final AuthoringProperties properties = new AuthoringProperties();
|
||||
private RuntimeExecutionContext context(Object selection) {
|
||||
return new RuntimeExecutionContext(1L, Path.of("dummy-workspace").toAbsolutePath(),
|
||||
selection == null ? Map.of() : Map.of("envAllowlist", selection), List.of(), null);
|
||||
}
|
||||
@Test void onlySelectedDefinedDummyValuesAreResolved() {
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of(DEMO, "AUTHORING_UNDEFINED", "AUTHORING_OTHER"));
|
||||
var calls = new ArrayList<String>();
|
||||
var adapter = adapter(calls);
|
||||
var result = adapter.scrubEnvironment(context(List.of(DEMO, "AUTHORING_UNDEFINED")));
|
||||
assertThat(result).containsEntry(DEMO, "dummy-value").containsEntry("PATH", "/dummy/bin")
|
||||
.containsEntry("LANG", "C.UTF-8").containsEntry("HOME", context(null).workingDirectory().toString())
|
||||
.doesNotContainKeys("AUTHORING_UNDEFINED", "AUTHORING_OTHER");
|
||||
assertThat(calls).containsExactly("PATH", DEMO, "AUTHORING_UNDEFINED");
|
||||
}
|
||||
@Test void emptyBindingGetsOnlyTrustedStartupEnvironment() {
|
||||
var calls = new ArrayList<String>();
|
||||
var result = adapter(calls).scrubEnvironment(context(null));
|
||||
assertThat(result.keySet()).containsExactly("PATH", "HOME", "LANG");
|
||||
assertThat(calls).containsExactly("PATH");
|
||||
}
|
||||
@Test void unauthorizedMixedSelectionIsRejectedBeforeAnyLookup() {
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of(DEMO));
|
||||
var calls = new ArrayList<String>();
|
||||
var adapter = adapter(calls);
|
||||
assertThatThrownBy(() -> adapter.scrubEnvironment(context(List.of(DEMO, "AUTHORING_DENIED"))))
|
||||
.isInstanceOf(IllegalArgumentException.class).hasMessageNotContaining("dummy-value");
|
||||
assertThat(calls).isEmpty();
|
||||
}
|
||||
@Test void savedBindingIsRejectedAfterEffectiveRevocation() {
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of(DEMO));
|
||||
var calls = new ArrayList<String>();
|
||||
var adapter = adapter(calls);
|
||||
var saved = context(List.of(DEMO));
|
||||
assertThat(adapter.scrubEnvironment(saved)).containsKey(DEMO);
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of());
|
||||
calls.clear();
|
||||
assertThatThrownBy(() -> adapter.scrubEnvironment(saved)).isInstanceOf(IllegalArgumentException.class);
|
||||
assertThat(calls).isEmpty();
|
||||
}
|
||||
@Test void reservedAliasesNeverReachLookup() {
|
||||
var calls = new ArrayList<String>();
|
||||
var adapter = adapter(calls);
|
||||
for (String name : List.of("PATH", "Path", "path", "HOME", "LANG", "SYSTEMROOT", "SystemRoot")) {
|
||||
assertThatThrownBy(() -> adapter.scrubEnvironment(context(List.of(name))))
|
||||
.isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
assertThat(calls).isEmpty();
|
||||
}
|
||||
@Test void bothBackendModesRejectBeforeAvailabilityProbeOrPlanning() {
|
||||
for (var mode : AuthoringProperties.ScriptExecutionMode.values()) {
|
||||
properties.getLocalScript().setExecutionMode(mode);
|
||||
String backend = mode == AuthoringProperties.ScriptExecutionMode.DOCKER ? "docker" : "inline";
|
||||
ScriptCommandBuilder builder = new ScriptCommandBuilder() {
|
||||
public String backend() { return backend; }
|
||||
public boolean available() { throw new AssertionError("must not probe backend"); }
|
||||
public ScriptExecutionPlan plan(Path w, String i, Path s, List<String> a, Map<String,String> e, String t) {
|
||||
throw new AssertionError("must not plan backend");
|
||||
}
|
||||
};
|
||||
var adapter = new LocalScriptRuntimeAdapter(properties, List.of(builder),
|
||||
new ScriptEnvironmentPolicy(() -> properties.getLocalScript().getEnvironmentAllowlist()),
|
||||
name -> { throw new AssertionError("must not look up environment"); });
|
||||
assertThatThrownBy(() -> adapter.execute(context(List.of(DEMO)), null, null))
|
||||
.isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
}
|
||||
@Test void inlinePlanReceivesOnlyAuthorizedMap() {
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of(DEMO));
|
||||
var ctx = context(List.of(DEMO));
|
||||
var environment = adapter(new ArrayList<>()).scrubEnvironment(ctx);
|
||||
var plan = new InlineScriptCommandBuilder().plan(ctx.workingDirectory(), "sh",
|
||||
ctx.workingDirectory().resolve("check.sh"), List.of(), environment, "dummy");
|
||||
assertThat(plan.environment()).containsEntry(DEMO, "dummy-value").hasSize(4);
|
||||
assertThat(plan.command()).hasSize(2);
|
||||
}
|
||||
@Test void dockerPlanForwardsOnlyAuthorizedMapWithoutStartingContainer() {
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of(DEMO));
|
||||
var ctx = context(List.of(DEMO));
|
||||
var environment = adapter(new ArrayList<>()).scrubEnvironment(ctx);
|
||||
var plan = new DockerScriptCommandBuilder(properties).plan(workspace, "sh",
|
||||
workspace.resolve("check.sh"), List.of(), environment, "dummy");
|
||||
assertThat(plan.environment()).isNull(); // CLI control environment remains inherited.
|
||||
assertThat(plan.command()).contains(DEMO + "=dummy-value", "HOME=/workspace", "--read-only");
|
||||
assertThat(plan.command()).doesNotContain("AUTHORING_UNDEFINED=", "AUTHORING_DENIED=");
|
||||
assertThat(plan.command().stream().filter("-e"::equals).count()).isEqualTo(4);
|
||||
}
|
||||
private LocalScriptRuntimeAdapter adapter(List<String> calls) {
|
||||
return new LocalScriptRuntimeAdapter(properties, List.of(),
|
||||
new ScriptEnvironmentPolicy(() -> properties.getLocalScript().getEnvironmentAllowlist()),
|
||||
name -> {
|
||||
calls.add(name);
|
||||
return switch (name) { case "PATH" -> "/dummy/bin"; case DEMO -> "dummy-value"; default -> null; };
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,108 @@
|
|||
package com.iflytek.skillhub.service.authoring.adapter;
|
||||
|
||||
import com.iflytek.skillhub.config.AuthoringProperties;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.RuntimeEventSink;
|
||||
import com.iflytek.skillhub.domain.authoring.runtime.RuntimeExecutionContext;
|
||||
import com.iflytek.skillhub.domain.authoring.service.ScriptEnvironmentPolicy;
|
||||
import com.iflytek.skillhub.domain.authoring.spec.TaskType;
|
||||
import com.iflytek.skillhub.domain.authoring.spec.ValidationTaskSpec;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.condition.EnabledOnOs;
|
||||
import org.junit.jupiter.api.condition.OS;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
@EnabledOnOs({OS.LINUX, OS.MAC})
|
||||
class LocalScriptEnvironmentExecutionTest {
|
||||
|
||||
@TempDir
|
||||
Path workspace;
|
||||
|
||||
@Test
|
||||
void realScriptReceivesSelectedSyntheticValueAndStreamsItsOutput() throws Exception {
|
||||
Files.writeString(workspace.resolve("check.sh"), """
|
||||
printf '%s|%s' "$AUTHORING_DEMO" "${AUTHORING_UNSELECTED-unset}" > executed.txt
|
||||
printf '%s\\n' "$AUTHORING_DEMO"
|
||||
""");
|
||||
var lookups = new ArrayList<String>();
|
||||
var sink = new RecordingSink();
|
||||
|
||||
var result = adapter(lookups).execute(context(List.of("AUTHORING_DEMO")), task(), sink);
|
||||
|
||||
assertThat(result.exitCode()).isZero();
|
||||
assertThat(result.stdout()).isEqualTo("synthetic-review-value");
|
||||
assertThat(result.stderr()).isEmpty();
|
||||
assertThat(Files.readString(workspace.resolve("executed.txt")))
|
||||
.isEqualTo("synthetic-review-value|unset");
|
||||
assertThat(sink.logs).containsExactly("synthetic-review-value");
|
||||
assertThat(lookups).containsExactly("PATH", "AUTHORING_DEMO");
|
||||
}
|
||||
|
||||
@Test
|
||||
void deniedSelectionDoesNotStartTheRealScriptOrLookUpValues() throws Exception {
|
||||
Files.writeString(workspace.resolve("check.sh"), "printf 'started' > executed.txt\n");
|
||||
var lookups = new ArrayList<String>();
|
||||
var sink = new RecordingSink();
|
||||
var adapter = adapter(lookups);
|
||||
|
||||
assertThatThrownBy(() -> adapter.execute(
|
||||
context(List.of("AUTHORING_DEMO", "AUTHORING_DENIED")), task(), sink))
|
||||
.isInstanceOf(IllegalArgumentException.class);
|
||||
|
||||
assertThat(workspace.resolve("executed.txt")).doesNotExist();
|
||||
assertThat(lookups).isEmpty();
|
||||
assertThat(sink.logs).isEmpty();
|
||||
assertThat(sink.toolCalls).isZero();
|
||||
}
|
||||
|
||||
private LocalScriptRuntimeAdapter adapter(List<String> lookups) {
|
||||
var properties = new AuthoringProperties();
|
||||
properties.getLocalScript().setExecutionMode(AuthoringProperties.ScriptExecutionMode.INLINE);
|
||||
properties.getLocalScript().setEnvironmentAllowlist(List.of("AUTHORING_DEMO", "AUTHORING_UNSELECTED"));
|
||||
return new LocalScriptRuntimeAdapter(properties, List.of(new InlineScriptCommandBuilder()),
|
||||
new ScriptEnvironmentPolicy(() -> properties.getLocalScript().getEnvironmentAllowlist()),
|
||||
name -> {
|
||||
lookups.add(name);
|
||||
return "PATH".equals(name) ? "/usr/bin:/bin" : "synthetic-review-value";
|
||||
});
|
||||
}
|
||||
|
||||
private RuntimeExecutionContext context(List<String> selection) {
|
||||
return new RuntimeExecutionContext(1L, workspace,
|
||||
Map.of("interpreter", "sh", "envAllowlist", selection), List.of(), null);
|
||||
}
|
||||
|
||||
private ValidationTaskSpec task() {
|
||||
return new ValidationTaskSpec("environment-check", "", TaskType.SCRIPT,
|
||||
"check.sh", List.of(), null, 5_000, List.of());
|
||||
}
|
||||
|
||||
private static final class RecordingSink implements RuntimeEventSink {
|
||||
private final List<String> logs = Collections.synchronizedList(new ArrayList<>());
|
||||
private int toolCalls;
|
||||
|
||||
@Override
|
||||
public void agentMessage(String taskName, String content) { }
|
||||
|
||||
@Override
|
||||
public void toolCall(String taskName, String tool, String argumentsJson) {
|
||||
toolCalls++;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void toolResult(String taskName, String tool, String summaryJson) { }
|
||||
|
||||
@Override
|
||||
public void log(String taskName, String stream, String line) {
|
||||
logs.add(line);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -29,7 +29,10 @@ public class RuntimeBindingValidator {
|
|||
private static final List<String> TRANSPORTS = List.of("http", "sse", "stdio");
|
||||
private static final List<String> INTERPRETERS = List.of("sh", "bash", "python3", "node");
|
||||
|
||||
public RuntimeBindingValidator() {
|
||||
private final ScriptEnvironmentPolicy environmentPolicy;
|
||||
|
||||
public RuntimeBindingValidator(ScriptEnvironmentPolicy environmentPolicy) {
|
||||
this.environmentPolicy = environmentPolicy;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -83,11 +86,17 @@ public class RuntimeBindingValidator {
|
|||
Object envAllowlist = config.get("envAllowlist");
|
||||
if (envAllowlist instanceof List<?> entries) {
|
||||
for (Object entry : entries) {
|
||||
if (entry == null || !ENV_REF_NAME.matcher(entry.toString()).matches()) {
|
||||
if (!(entry instanceof String name) || !ENV_REF_NAME.matcher(name).matches()) {
|
||||
findings.add(FindingDraft.error(ValidationLayer.CONFIG, "ENV_ALLOWLIST_INVALID",
|
||||
"Invalid environment variable name: " + entry));
|
||||
}
|
||||
}
|
||||
try {
|
||||
environmentPolicy.authorize(entries);
|
||||
} catch (IllegalArgumentException exception) {
|
||||
findings.add(FindingDraft.error(ValidationLayer.CONFIG, "ENV_ALLOWLIST_FORBIDDEN",
|
||||
"Environment selection is not permitted by deployment policy"));
|
||||
}
|
||||
} else if (envAllowlist != null) {
|
||||
findings.add(FindingDraft.error(ValidationLayer.CONFIG, "ENV_ALLOWLIST_INVALID",
|
||||
"envAllowlist must be a list of variable names"));
|
||||
|
|
|
|||
|
|
@ -0,0 +1,61 @@
|
|||
package com.iflytek.skillhub.domain.authoring.service;
|
||||
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Set;
|
||||
import java.util.function.Supplier;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
/** Deployment-owned ceiling; a draft may only select a subset, never grant access. */
|
||||
public final class ScriptEnvironmentPolicy {
|
||||
|
||||
private static final Pattern NAME = Pattern.compile("^[A-Z][A-Z0-9_]{0,63}$");
|
||||
private static final Set<String> RESERVED = Set.of(
|
||||
"PATH", "HOME", "LANG", "SYSTEMROOT", "WINDIR", "COMSPEC", "PATHEXT", "TEMP", "TMP");
|
||||
private final Supplier<List<String>> deploymentAllowlist;
|
||||
|
||||
public ScriptEnvironmentPolicy(Supplier<List<String>> deploymentAllowlist) {
|
||||
this.deploymentAllowlist = java.util.Objects.requireNonNull(deploymentAllowlist);
|
||||
allowedNames();
|
||||
}
|
||||
|
||||
/** Reads the currently effective deployment policy on every call, including execution. */
|
||||
public Set<String> authorize(Object requested) {
|
||||
Set<String> allowed = allowedNames();
|
||||
if (requested == null) {
|
||||
return Set.of();
|
||||
}
|
||||
if (!(requested instanceof List<?> entries)) {
|
||||
throw new IllegalArgumentException("envAllowlist must be a list of variable names");
|
||||
}
|
||||
Set<String> selected = new LinkedHashSet<>();
|
||||
for (Object entry : entries) {
|
||||
if (!(entry instanceof String name) || !validName(name) || !allowed.contains(name)) {
|
||||
throw new IllegalArgumentException(
|
||||
"envAllowlist contains a variable not permitted by deployment policy");
|
||||
}
|
||||
selected.add(name);
|
||||
}
|
||||
return java.util.Collections.unmodifiableSet(selected);
|
||||
}
|
||||
|
||||
private Set<String> allowedNames() {
|
||||
List<String> configured = deploymentAllowlist.get();
|
||||
if (configured == null) {
|
||||
return Set.of();
|
||||
}
|
||||
Set<String> allowed = new LinkedHashSet<>();
|
||||
for (String name : configured) {
|
||||
if (name == null || !validName(name)) {
|
||||
throw new IllegalStateException("Invalid local-script deployment environment allowlist");
|
||||
}
|
||||
allowed.add(name);
|
||||
}
|
||||
return Set.copyOf(allowed);
|
||||
}
|
||||
|
||||
private static boolean validName(String name) {
|
||||
return NAME.matcher(name).matches() && !RESERVED.contains(name.toUpperCase(Locale.ROOT));
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
package com.iflytek.skillhub.domain.authoring.service;
|
||||
|
||||
import com.iflytek.skillhub.domain.authoring.*;
|
||||
import com.iflytek.skillhub.domain.shared.exception.*;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import static org.assertj.core.api.Assertions.*;
|
||||
import static org.mockito.Mockito.*;
|
||||
import static org.mockito.ArgumentMatchers.*;
|
||||
|
||||
class RuntimeBindingEnvironmentAuthorizationTest {
|
||||
@Test void ordinaryDraftOwnerCanSaveOnlyDeploymentApprovedSelection() {
|
||||
var bindings = mock(RuntimeBindingRepository.class);
|
||||
var drafts = mock(SkillDraftRepository.class);
|
||||
var policy = new ScriptEnvironmentPolicy(() -> List.of("AUTHORING_DEMO"));
|
||||
var service = new RuntimeBindingService(bindings, drafts, new RuntimeBindingValidator(policy));
|
||||
when(drafts.findById(7L)).thenReturn(Optional.of(new SkillDraft(1L, "ordinary-member", "test", "test", "digest")));
|
||||
when(bindings.findByDraftId(7L)).thenReturn(Optional.empty());
|
||||
when(bindings.save(any())).thenAnswer(call -> call.getArgument(0));
|
||||
var result = service.saveBinding(7L, "ordinary-member", "local-script",
|
||||
Map.of("envAllowlist", List.of("AUTHORING_DEMO")), null, null, Set.of());
|
||||
assertThat(result.getConfig()).containsKey("envAllowlist");
|
||||
verify(bindings).save(any());
|
||||
clearInvocations(bindings);
|
||||
assertThatThrownBy(() -> service.saveBinding(7L, "ordinary-member", "local-script",
|
||||
Map.of("envAllowlist", List.of("AUTHORING_OTHER")), null, null, Set.of()))
|
||||
.isInstanceOf(DomainBadRequestException.class);
|
||||
verify(bindings, never()).save(any());
|
||||
}
|
||||
@Test void superAdminRoleDoesNotExpandDeploymentGrant() {
|
||||
var bindings = mock(RuntimeBindingRepository.class);
|
||||
var drafts = mock(SkillDraftRepository.class);
|
||||
when(drafts.findById(7L)).thenReturn(Optional.of(new SkillDraft(1L, "ordinary-member", "test", "test", "digest")));
|
||||
var service = new RuntimeBindingService(bindings, drafts,
|
||||
new RuntimeBindingValidator(new ScriptEnvironmentPolicy(List::of)));
|
||||
assertThatThrownBy(() -> service.saveBinding(7L, "admin", "local-script",
|
||||
Map.of("envAllowlist", List.of("AUTHORING_DEMO")), null, null, Set.of("SUPER_ADMIN")))
|
||||
.isInstanceOf(DomainBadRequestException.class);
|
||||
verify(bindings, never()).save(any());
|
||||
}
|
||||
@Test void nonOwnerCannotUseAnOtherwiseAuthorizedSelection() {
|
||||
var bindings = mock(RuntimeBindingRepository.class);
|
||||
var drafts = mock(SkillDraftRepository.class);
|
||||
when(drafts.findById(7L)).thenReturn(Optional.of(new SkillDraft(1L, "owner", "test", "test", "digest")));
|
||||
var service = new RuntimeBindingService(bindings, drafts,
|
||||
new RuntimeBindingValidator(new ScriptEnvironmentPolicy(() -> List.of("AUTHORING_DEMO"))));
|
||||
assertThatThrownBy(() -> service.saveBinding(7L, "other-member", "local-script",
|
||||
Map.of("envAllowlist", List.of("AUTHORING_DEMO")), null, null, Set.of()))
|
||||
.isInstanceOf(DomainForbiddenException.class);
|
||||
verify(bindings, never()).save(any());
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,70 @@
|
|||
package com.iflytek.skillhub.domain.authoring.service;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import static org.assertj.core.api.Assertions.*;
|
||||
|
||||
class ScriptEnvironmentPolicyTest {
|
||||
@Test void emptyDeploymentDeniesRequestedVariable() {
|
||||
var policy = new ScriptEnvironmentPolicy(List::of);
|
||||
assertThat(policy.authorize(null)).isEmpty();
|
||||
assertThat(policy.authorize(List.of())).isEmpty();
|
||||
assertThatThrownBy(() -> policy.authorize(List.of("AUTHORING_DEMO")))
|
||||
.isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
@Test void acceptsOnlySelectedSubsetAndDeduplicates() {
|
||||
var policy = new ScriptEnvironmentPolicy(() -> List.of("AUTHORING_DEMO", "AUTHORING_OTHER"));
|
||||
assertThat(policy.authorize(List.of("AUTHORING_DEMO", "AUTHORING_DEMO")))
|
||||
.containsExactly("AUTHORING_DEMO");
|
||||
assertThat(policy.authorize(List.of())).isEmpty();
|
||||
}
|
||||
@Test void mixedSelectionIsRejectedAsAWhole() {
|
||||
var policy = new ScriptEnvironmentPolicy(() -> List.of("AUTHORING_DEMO"));
|
||||
assertThatThrownBy(() -> policy.authorize(List.of("AUTHORING_DEMO", "AUTHORING_OTHER")))
|
||||
.isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
@Test void malformedAndNonStringNamesCannotCoerceOrMatch() {
|
||||
var policy = new ScriptEnvironmentPolicy(() -> List.of("AUTHORING_DEMO"));
|
||||
for (Object value : List.of(42, Map.of("name", "AUTHORING_DEMO"), "authoring_demo", " AUTHORING_DEMO", "AUTHORING_DEMO ", "A-B", "A".repeat(65))) {
|
||||
assertThatThrownBy(() -> policy.authorize(List.of(value))).isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
assertThatThrownBy(() -> policy.authorize(Arrays.asList((Object) null))).isInstanceOf(IllegalArgumentException.class);
|
||||
assertThatThrownBy(() -> policy.authorize("AUTHORING_DEMO")).isInstanceOf(IllegalArgumentException.class);
|
||||
}
|
||||
@Test void reservedNamesCannotBeAuthorizedByDeployment() {
|
||||
for (String reserved : List.of("PATH", "HOME", "LANG", "SYSTEMROOT", "WINDIR", "COMSPEC", "PATHEXT", "TEMP", "TMP", "Path", "path")) {
|
||||
assertThatThrownBy(() -> new ScriptEnvironmentPolicy(() -> List.of(reserved)))
|
||||
.isInstanceOf(IllegalStateException.class);
|
||||
}
|
||||
}
|
||||
@Test void malformedDeploymentFailsClosed() {
|
||||
assertThatThrownBy(() -> new ScriptEnvironmentPolicy(() -> List.of("bad-name")))
|
||||
.isInstanceOf(IllegalStateException.class);
|
||||
assertThatThrownBy(() -> new ScriptEnvironmentPolicy(() -> Arrays.asList((String) null)))
|
||||
.isInstanceOf(IllegalStateException.class);
|
||||
assertThat(new ScriptEnvironmentPolicy(() -> null).authorize(null)).isEmpty();
|
||||
}
|
||||
@Test void effectiveRevocationAppliesToPreviouslyValidSelection() {
|
||||
var current = new AtomicReference<>(List.of("AUTHORING_DEMO"));
|
||||
var policy = new ScriptEnvironmentPolicy(current::get);
|
||||
var selection = List.of("AUTHORING_DEMO");
|
||||
assertThat(policy.authorize(selection)).containsExactly("AUTHORING_DEMO");
|
||||
current.set(List.of());
|
||||
assertThatThrownBy(() -> policy.authorize(selection)).isInstanceOf(IllegalArgumentException.class);
|
||||
current.set(List.of("PATH"));
|
||||
assertThatThrownBy(() -> policy.authorize(null)).isInstanceOf(IllegalStateException.class);
|
||||
}
|
||||
@Test void configValidationUsesCurrentPolicyWithoutDisclosingValues() {
|
||||
var current = new AtomicReference<>(List.of("AUTHORING_DEMO"));
|
||||
var validator = new RuntimeBindingValidator(new ScriptEnvironmentPolicy(current::get));
|
||||
var config = Map.<String, Object>of("interpreter", "sh", "envAllowlist", List.of("AUTHORING_DEMO"));
|
||||
assertThat(validator.validate("local-script", config, null, null)).isEmpty();
|
||||
current.set(List.of());
|
||||
assertThat(validator.validate("local-script", config, null, null))
|
||||
.extracting(finding -> finding.ruleCode()).containsExactly("ENV_ALLOWLIST_FORBIDDEN");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue