Unknown param-file keys silently become PARAM_* env vars — a mistyped or invented spend control caps nothing

[scttfrdmn]

Severity: medium — turns a class of typos and wrong guesses into silent misconfiguration

Any param-file key the sweep parser does not recognise becomes a PARAM_<key> environment
variable, with no warning. That is the correct behaviour for intentional user parameters —
it is how a sweep passes values to a workload — but it means the parser can never say "that
isn't a thing." A typo, a renamed key, or a plausible-sounding key that simply does not exist
all produce a launch that looks completely healthy and does something other than what the
file says.

cmd/sweep.go:188-190:

default:
    // All unknown fields become parameters (PARAM_* env vars)
    config.Parameters[key] = fmt.Sprintf("%v", val)

Concrete failures this allows, all of which launch and bill normally:

written in the param file	what the user believes	what happens
cost_limit: 8	a per-instance $8 cap	PARAM_cost_limit=8; no cap (#525)
budget: 50	a sweep budget	PARAM_budget=50; nothing
max_concurrent: 3	a concurrency cap	PARAM_max_concurrent=3; nothing (it is a CLI flag)
instance_types: [...]	several types	PARAM_instance_types; every row on the CLI default type
ttl_hours: 4	a TTL (misremembering ttl:)	PARAM_ttl_hours=4; no TTL
on-complete: terminate	hyphen instead of underscore	PARAM_on-complete; instance never terminates

The last two are the ones that cost money: the user wrote a lifecycle bound, the parser
accepted the file without complaint, and the instance has no bound.

Why file this separately from #525

So it isn't lost inside it. #525 is about two specific flags; this is the mechanism that
makes every future spend-control key fail the same way. Fixing it turns a class of silent
misconfigurations into loud errors, and it is the precondition for #525 option (b)
("set cost_limit: per row") being a real fix rather than a relocation of the problem.

Expected behaviour

PARAM_* passthrough must stay — it is the feature. What is missing is a way to distinguish
"a parameter for my workload" from "a spawn setting I got wrong." Options, roughly in order
of preference:

1. A reserved-name denylist that errors. Reject keys that are near-misses for real spawn
   controls or CLI-only flags — cost_limit, budget, max_concurrent, instance_types,
   ttl_hours, on-complete, and any recognised key written with hyphens instead of
   underscores. Error message should name the right spelling or say the control is CLI-only.
   Cheap, and it catches every row in the table above.
2. An explicit namespace for passthrough, e.g. params:/env: sub-map, with unknown
   top-level keys erroring. Cleanest, but breaking; needs a deprecation path.
3. At minimum, log every PARAM_* derived from an unrecognised key at launch time, so
   PARAM_cost_limit=8 appears in the output where a user might notice it. Weakest — it is
   still a warning nobody reads — but strictly better than silence and non-breaking.

(1) plus (3) together would be a good landing spot: loud failure on the known-dangerous
names, visible listing for the rest.

Note the related trap on the same surface: unknown keys at the top level of a param file
(outside defaults:/grid:/params:) are dropped entirely — spawn's own
examples/simple-params.yaml puts region:/instance_type:/ami: there, where they are
discarded. That is arguably a separate bug, but it is the same "unrecognised input accepted
in silence" pattern and probably wants the same fix.

Regression test to add

given  a param file with `cost_limit: 8` under defaults:
when   the params are parsed
then   parsing FAILS with an error naming cost_limit as unsupported
       (and NOT a config carrying Parameters["cost_limit"])

given  a param file with `my_workload_arg: 42`
when   the params are parsed
then   parsing succeeds and Parameters["my_workload_arg"] == "42"

The second case is as important as the first: the fix must not break intentional
passthrough, which is the whole point of the default: branch.

Cross-references

• --estimate-only is ignored on the foreground sweep path — --no-detach launches every row instead of estimating #524 — --estimate-only dropped on the sweep path.
• Sweep dispatch silently drops --cost-limit and --ttl, then logs "Using safeguards: ttl=..." #525 — --cost-limit/--ttl dropped on the sweep path. That issue's option (b)
  depends on this one.

Found while building a GROMACS benchmark sweep for the cost-to-result project (recorded
there as finding 15 in docs/platform-findings.md). A draft matrix in that project had
cost_limit: 8 under defaults: with the comment "the ENFORCED cap"; the parser accepted
it, and it capped nothing. That project's harness/check_matrix.py now carries a hardcoded
denylist of exactly these keys — a check that belongs upstream, in the parser, not in each
consumer that happens to have been burned.

[scttfrdmn]

Related and now partly precedented by #525 / #529: cost_limit has been promoted out of the default: arm into a real parser case, and that case errors on a value it cannot use rather than leaving the field at 0. So the first row of the table above is fixed specifically — but the mechanism this issue is about is untouched, and budget:, ttl_hours:, on-complete: and friends still launch silently.

Taking option (1) + (3) from the expected-behaviour list next: a denylist that errors on the known-dangerous near-misses (plus a generic rule for a recognised key written with hyphens or in the wrong case), and a visible listing of every PARAM_* derived from an unrecognised key so intentional passthrough stays intentional.

[scttfrdmn]

Fixed in #532 (stacked on #529 → #528).

Landed options (1) + (3) from the expected-behaviour list, plus one thing the issue did not anticipate.

(1) Rejection has three rules, checked in order: a recognised key misspelled by hyphen or case; a curated denylist of CLI-only flags and near-misses, each carrying the right spelling; and — the mechanical one — a key that is not a valid shell identifier. That last rule is grounded in pkg/launcher/bootstrap.go:534, which writes export PARAM_${param_name}=\"${value}\" into /etc/profile.d: export PARAM_on-complete=\"terminate\" is a "not a valid identifier" error in every login shell, so a hyphenated key was never reaching the workload whatever it meant. All offending keys report in one error, not one per run.

(3) the PARAM_* variables a sweep will set are listed in the sweep header.

The thing the issue missed: a denylist with no escape hatch is a hard block, not a redirect. budget is an optimiser's evaluation budget as often as it is a dollar figure, and time_limit is a standard solver option — with no way through, this check would have broken the feature it exists to protect. So param:<name>: passes any name straight through as PARAM_<name>, and every rejection message names that remedy.

timeout is not on the denylist, and that is deliberate: the first draft rejected it and TestBuildLaunchConfigFromParams_WorkflowStep failed. examples/workflow-ci-pipeline.yaml sets timeout: 10m per step and pkg/queue.JobConfig has a Timeout field — documented vocabulary, not a near-miss. Same reasoning excludes image, type, count, steps, instance and runtime; all are pinned as allowed by a test.

The recognised-key registry duplicates the switch it mirrors, so a go/ast test extracts the case labels from the source and requires equality in both directions — including a guard that fails if the walk finds zero labels, since a broken walk would make the rest pass vacuously.

Your closing note is filed as #530 (top-level keys dropped entirely — and spawn's own examples/simple-params.yaml and examples/schedule-params.yaml both do it, with cmd/schedule.go reading those same names out of defaults:). The value half of that export line is #531 (unquoted interpolation, so $HOME/out expands on the instance).