konveyor / konveyor/agentic-controller
spec.env / spec.envFrom can turn on ask_user (and retune HITL) behind spec.execution
- Dominant language
- Go
- Stars
- 2
- Forks
- 12
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 41
Description
## Problem
#242 made `ask_user` opt-in through a typed field, `spec.execution.askUser`, so that "can this run be failed by an unanswered question?" is answerable by reading the run's spec. It isn't, yet: the harness still honours the env var the field replaced, and the controller passes user env straight to the harness container.
`harness/internal/config/config.go`:
```go
cfg.HITLAsk = (cfg.Params.Execution.AskUser || envSwitchedOn("HARNESS_HITL_ASK")) &&
!envSwitchedOff("HARNESS_HITL_ASK")
```
`internal/controller/agentrun_controller.go` (end of the env builder) appends `run.Spec.Env` last, filtering only the two git-identity names; `run.Spec.EnvFrom` is appended to the container unfiltered.
So this run gets the `ask_user` tool with `spec.execution.askUser` unset:
```yaml
kind: AgentRun
spec:
agentRef: migrate
env:
- name: HARNESS_HITL_ASK
value: "on"
```
The same goes for the rest of the harness's switches — `HARNESS_HITL_TIMEOUT_SECONDS`, `HARNESS_HITL_STEER`, `HARNESS_ACP_TEE` — and, because user env is appended last and last-key-wins, for controller-set `KONVEYOR_*` vars too (`KONVEYOR_PROMPT`, `KONVEYOR_SKILL_SOURCES`, …).
## Why it matters
- **The spec stops being the record.** `resolveExecution` deliberately keeps the stored `ExecutionSpec` "a faithful record of what was set". A run whose `execution` says nothing about `askUser` but which has the tool mounted is exactly the ROKS failure from #242 again, now undiagnosable from `kubectl get agentrun -o yaml | grep askUser`.
- **It bypasses whatever governs `execution`.** ADR 0018 puts "who may set or raise limits" on Hub/UI. Anything Hub decides about `askUser` (or a future per-run timeout) is moot if `env` is an equivalent, ungoverned path. Hub already accepts `spec.env` from the console (`appContextEnv`), so this is reachable through the product today, not only via `kubectl`.
- **`off` is a silent override in the other direction.** `HARNESS_HITL_ASK=off` in an `envFrom` ConfigMap makes `askUser: true` a no-op with nothing in status to say so.
Not a privilege-escalation report — anyone who can set `env` can already create the run. It's a "two control planes for one setting" problem, and the typed one should win.
## Precedent
The controller already does this for one pair of names. From `agentrun_controller.go`:
> gitAuthorNameEnv / gitAuthorEmailEnv … are controller-managed: a user copy in run.Spec.Env is dropped, and the controller always emits both (even empty) so container env outranks any copy smuggled in through run.Spec.EnvFrom.
The proposal is to generalise that from two names to the namespaces the controller and harness own.
## Proposal
1. **Reserve `HARNESS_*` and `KONVEYOR_*` in `spec.env`.** Prefer rejecting at admission (CEL rule on the CRD, or the validating webhook if there is one) over silently dropping — a dropped var is another invisible behaviour. If rejection is too breaking, drop and set a `Warning` event/condition naming the dropped keys.
2. **Close the `envFrom` path for the switches that matter** the same way git identity does: have the controller always emit an explicit `HARNESS_HITL_ASK` derived from the resolved spec (`on` when `askUser`, `off` otherwise). Container env outranks `envFrom`, and `off` already wins in the harness, so no harness change is needed.
3. **Keep the env vars for what they were kept for** — running the harness outside the controller. Nothing above touches that.
4. If a per-run HITL timeout is a real need, add it to `ExecutionSpec` rather than leaving `HARNESS_HITL_TIMEOUT_SECONDS` as the way to get it.
## Open questions
- Does anything legitimately set `KONVEYOR_*` via `spec.env` today? The console's `appContextEnv` is the one to check before reserving the whole prefix; an allowlist of user-settable `KONVEYOR_*` names may be needed.
- Stage AgentRuns created by the AgentWorkflowRun controller: confirm they take the same env builder so the rule applies once.
## Acceptance
- An AgentRun with `HARNESS_HITL_ASK` (or any reserved name) in `spec.env` is rejected, or the var is dropped with a visible warning.
- With `askUser` unset and `HARNESS_HITL_ASK=on` supplied via an `envFrom` ConfigMap, the harness does not mount `ask_user`.
- With `askUser: true`, the tool is mounted regardless of `envFrom` contents.
- `HARNESS_HITL_ASK=on` still works for a harness run outside the controller.
- ADR 0017's 2026-09-17 update and `harness/README.md` say the env var is standalone-harness only.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with harness/internal/config/config.go and internal/controller/agentrun_controller.go, tracing how execution settings and user env reach the harness container. Check the console's appContextEnv and AgentWorkflowRun path for affected env sources, then review ADR 0017 and harness/README.md. Done means the listed acceptance cases pass and the standalone-harness distinction is documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- backend, infrastructure
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100