From 57eb1846bdbb867a37bc1b5dc468e6f1b2fe02dc Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 05:22:40 +0000 Subject: [PATCH 1/2] fix(analysis): refuse malformed OPENSYSML_TOOL_ENV_PASSTHROUGH names Co-Authored-By: jason.han --- .../tool-env-passthrough-names.fixed.md | 1 + docs/reference/environment.md | 2 +- internal/exec/analysis/invocation.go | 3 +++ internal/exec/analysis/invocation_test.go | 22 +++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 changes/unreleased/tool-env-passthrough-names.fixed.md diff --git a/changes/unreleased/tool-env-passthrough-names.fixed.md b/changes/unreleased/tool-env-passthrough-names.fixed.md new file mode 100644 index 000000000..a5327ae5b --- /dev/null +++ b/changes/unreleased/tool-env-passthrough-names.fixed.md @@ -0,0 +1 @@ +- **`OPENSYSML_TOOL_ENV_PASSTHROUGH` refuses malformed entries.** An entry containing `=` or a NUL byte is not an environment variable name; the tool's invocation now fails with an error naming the variable and the entry instead of forwarding it. diff --git a/docs/reference/environment.md b/docs/reference/environment.md index fdb65dc49..24296069d 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -23,7 +23,7 @@ run that would never finish into a reported error instead of a hang. | `OPENSYSML_ENGINES` | unset (no engines) | Directory of the **engine manifest**: one JSON file per external analysis engine — a model checker, a simulator, a solver — spoken to over its standard input by the protocol on [External engines](external-engines.md), listed by `-engines` and selected by `-engine ` like any engine of the build. Read beside `OPENSYSML_TOOLS` under the same rules; see [External engines](#external-engines) | | `OPENSYSML_TOOL_TIMEOUT` | `10s` | How long one tool process may take, as a Go duration (`5s`, `500ms`), after which the performance fails with a timeout; for an external engine, how long its `describe` and `covers` may take and the grace a `run` has to answer `cancel` before its process is ended. A value that is not a positive duration is the default | | `OPENSYSML_TOOL_MAX_OUTPUT` | `64M` | How much one external process may write before it is cut off: a tool's one reply and the whole of its standard error, an external engine's one protocol line and the whole of its standard error. Bytes, or bytes with a `K`, `M` or `G` suffix; a value that is not a positive size is the default | -| `OPENSYSML_TOOL_ENV_PASSTHROUGH` | unset | Comma-separated names of this process's environment variables handed to a tool started under a manifest [`invocation`](#external-tools) block, beside `PATH`, `HOME`, `TMPDIR` and `LANG`; nothing else of the environment reaches such a tool. A tool without the block inherits the whole environment as before | +| `OPENSYSML_TOOL_ENV_PASSTHROUGH` | unset | Comma-separated names of this process's environment variables handed to a tool started under a manifest [`invocation`](#external-tools) block, beside `PATH`, `HOME`, `TMPDIR` and `LANG`; nothing else of the environment reaches such a tool. An entry containing `=` or a NUL byte is not an environment variable name and is refused. A tool without the block inherits the whole environment as before | | `OPENSYSML_TOOL_KEEP` | unset | `1` keeps the per-invocation directory a tool's `{inputFile}` and `{outputDir}` name, for inspection; otherwise it is removed once the reply is read | | `OPENSYSML_GRPC_INDEX_POOL` | `4` | Whether `sysml-grpc` builds the one shared standard library index ahead of the requests needing it; any positive value prewarms, `0` builds it on the first request instead | | `OPENSYSML_GRPC_MAX_HELD_OBJECTS` | `10000` | The most objects `sysml-grpc` keeps for one cached model, nested objects counted: `Instantiate` creates them and document queries bind and enumerate them for as long as the model stays cached. An `Instantiate`, query or render whose objects would pass the bound fails whole with `RESOURCE_EXHAUSTED`, leaving none of them, until the model leaves the cache, which releases them together; nothing is evicted behind an id a client holds. Read at startup | diff --git a/internal/exec/analysis/invocation.go b/internal/exec/analysis/invocation.go index e0e7f9fcd..b1318e24f 100644 --- a/internal/exec/analysis/invocation.go +++ b/internal/exec/analysis/invocation.go @@ -527,6 +527,9 @@ func (inv *Invocation) renderEnv(sc scope) ([]string, error) { names := append([]string(nil), baseToolEnv...) for _, name := range strings.Split(os.Getenv(ToolEnvPassthroughEnv), ",") { if name = strings.TrimSpace(name); name != "" { + if strings.ContainsAny(name, "=\x00") { + return nil, fmt.Errorf("%s lists %q, which is not an environment variable name", ToolEnvPassthroughEnv, name) + } names = append(names, name) } } diff --git a/internal/exec/analysis/invocation_test.go b/internal/exec/analysis/invocation_test.go index 9c1223cf0..afb6785f1 100644 --- a/internal/exec/analysis/invocation_test.go +++ b/internal/exec/analysis/invocation_test.go @@ -372,6 +372,28 @@ func TestInvocationEnvIsMinimal(t *testing.T) { } } +// A name OPENSYSML_TOOL_ENV_PASSTHROUGH lists that carries `=` or a NUL byte is not an +// environment variable name: the refusal names the variable and the entry. Whitespace and +// empty entries around the names are still tolerated. +func TestInvocationEnvPassthroughRefusesMalformedNames(t *testing.T) { + t.Setenv("OPENSYSML_TEST_SHOWN", "shown") + inv := checked(t, &Invocation{}, "label") + sc := scope{tool: "Solver", inputs: map[string]runtime.ToolValue{"label": {Text: "r1"}}} + + t.Setenv(ToolEnvPassthroughEnv, " OPENSYSML_TEST_SHOWN ,") + if _, err := inv.renderEnv(sc); err != nil { + t.Fatalf("a valid name: %v", err) + } + for _, bad := range []string{"BAD=VALUE", "BAD = VALUE"} { + t.Setenv(ToolEnvPassthroughEnv, " OPENSYSML_TEST_SHOWN , "+bad+" ,") + _, err := inv.renderEnv(sc) + want := fmt.Sprintf("%s lists %q, which is not an environment variable name", ToolEnvPassthroughEnv, strings.TrimSpace(bad)) + if err == nil || err.Error() != want { + t.Errorf("renderEnv: %v, want %q", err, want) + } + } +} + func sortedPairs(env []string) bool { for i := 1; i < len(env); i++ { if env[i-1] > env[i] { From 351aa89dc06983d5e7f4ac229c069d8188fb38bf Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 07:01:41 +0000 Subject: [PATCH 2/2] fix(analysis): type the malformed passthrough-name error Co-Authored-By: jason.han --- internal/exec/analysis/invocation.go | 3 ++- internal/exec/analysis/invocation_test.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/exec/analysis/invocation.go b/internal/exec/analysis/invocation.go index b1318e24f..465817b6e 100644 --- a/internal/exec/analysis/invocation.go +++ b/internal/exec/analysis/invocation.go @@ -528,7 +528,8 @@ func (inv *Invocation) renderEnv(sc scope) ([]string, error) { for _, name := range strings.Split(os.Getenv(ToolEnvPassthroughEnv), ",") { if name = strings.TrimSpace(name); name != "" { if strings.ContainsAny(name, "=\x00") { - return nil, fmt.Errorf("%s lists %q, which is not an environment variable name", ToolEnvPassthroughEnv, name) + return nil, &runtime.ToolError{Tool: sc.tool, Kind: runtime.ToolProcessFailed, + Detail: fmt.Sprintf("%s lists %q, which is not an environment variable name", ToolEnvPassthroughEnv, name)} } names = append(names, name) } diff --git a/internal/exec/analysis/invocation_test.go b/internal/exec/analysis/invocation_test.go index afb6785f1..520f08f37 100644 --- a/internal/exec/analysis/invocation_test.go +++ b/internal/exec/analysis/invocation_test.go @@ -388,8 +388,9 @@ func TestInvocationEnvPassthroughRefusesMalformedNames(t *testing.T) { t.Setenv(ToolEnvPassthroughEnv, " OPENSYSML_TEST_SHOWN , "+bad+" ,") _, err := inv.renderEnv(sc) want := fmt.Sprintf("%s lists %q, which is not an environment variable name", ToolEnvPassthroughEnv, strings.TrimSpace(bad)) - if err == nil || err.Error() != want { - t.Errorf("renderEnv: %v, want %q", err, want) + var fault *runtime.ToolError + if !errors.As(err, &fault) || fault.Tool != "Solver" || fault.Kind != runtime.ToolProcessFailed || fault.Detail != want { + t.Errorf("renderEnv: %v, want a ToolError{Tool: Solver, Kind: ToolProcessFailed, Detail: %q}", err, want) } } }