What was wrong:vaultik config set age_recipients.0 age1... on a file without age_recipients wrote a map keyed 0 and exited 0; afterwards no command could load the config.
What changed:
A missing key is created as a list when the next path element is a number, otherwise as a map. An index the new list cannot take, such as age_recipients.1, is an error.
Before writing, config set checks the result with the config loader and refuses only a failure the change brings in, leaving the file unchanged. A config that loaded must still load (compression_level 99 is refused). One that does not load, such as the config init file before storage_url is set, or one with a ${ENV:...} reference to a variable not set in this shell, is checked on the changed setting alone, inside the maps and lists leading to it: snapshots.home.paths /x is refused there too.
The new config.Parse, split out of config.Load, decodes config data, filling in ${...} references, without validating it.
config set --help and the README describe this.
Judgement call: the check reads the config twice per config set, so a ${EXEC:...} command can run twice.
Judgement call: a refusal for a value of the wrong type quotes up to ten characters of it, as the loader's error does.
Judgement call: a new value that is a ${...} reference not resolvable here is refused.
Rule suppressed: gosec G304 where config.Load and the new tests read a file.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/308.
**What was wrong:** `vaultik config set age_recipients.0 age1...` on a file without `age_recipients` wrote a map keyed `0` and exited 0; afterwards no command could load the config.
**What changed:**
- A missing key is created as a list when the next path element is a number, otherwise as a map. An index the new list cannot take, such as `age_recipients.1`, is an error.
- Before writing, `config set` checks the result with the config loader and refuses only a failure the change brings in, leaving the file unchanged. A config that loaded must still load (`compression_level 99` is refused). One that does not load, such as the `config init` file before `storage_url` is set, or one with a `${ENV:...}` reference to a variable not set in this shell, is checked on the changed setting alone, inside the maps and lists leading to it: `snapshots.home.paths /x` is refused there too.
- The new `config.Parse`, split out of `config.Load`, decodes config data, filling in `${...}` references, without validating it.
- `config set --help` and the README describe this.
Judgement call: the check reads the config twice per `config set`, so a `${EXEC:...}` command can run twice.
Judgement call: a refusal for a value of the wrong type quotes up to ten characters of it, as the loader's error does.
Judgement call: a new value that is a `${...}` reference not resolvable here is refused.
Rule suppressed: gosec G304 where `config.Load` and the new tests read a file.
Model: opus-5-5
internal/cli/config.go, writeConfigSet: the new check reads the whole file without filling in ${...} references the way config.Load does. On a config that loads and has a reference on a number, size or true/false setting (for example compression_level: ${ENV:LEVEL}, the kind of file the PR body says config edit can still write), every config set, on any key, now fails with "the config would no longer load", which is untrue. The same message appears when the file already failed to load before the change. Acceptable: refuse only a failure the change itself brings in (for example by also checking the file as it was, or by checking only the setting being changed), with an error that says what is actually wrong. Add a test that sets another key on a file holding such a reference.
The second point of the definition of done in #308 ("config set never writes a file the config loader rejects") is not met. On a config that loads, config set compression_level 99, config set chunk_size 1KB or config set age_recipients.0 notakey exits 0 and writes a file every later command refuses. The stated reason, that the file config init writes fails the loader's checks until storage_url is set, holds only for a config that is still incomplete. Acceptable: when the file passed the loader's checks before the change, the result must pass them too, or the change is refused and the file kept. A config that does not pass them yet keeps the type check alone. README and config set --help should say so, with a test.
The commit message body is about 140 words (limit about 120) and the PR body about 265 (limit about 250). Acceptable: shorten both.
Model: opus-5-5
1. `internal/cli/config.go`, `writeConfigSet`: the new check reads the whole file without filling in `${...}` references the way `config.Load` does. On a config that loads and has a reference on a number, size or true/false setting (for example `compression_level: ${ENV:LEVEL}`, the kind of file the PR body says `config edit` can still write), every `config set`, on any key, now fails with "the config would no longer load", which is untrue. The same message appears when the file already failed to load before the change. Acceptable: refuse only a failure the change itself brings in (for example by also checking the file as it was, or by checking only the setting being changed), with an error that says what is actually wrong. Add a test that sets another key on a file holding such a reference.
2. The second point of the definition of done in https://git.eeqj.de/sneak/vaultik/issues/308 ("`config set` never writes a file the config loader rejects") is not met. On a config that loads, `config set compression_level 99`, `config set chunk_size 1KB` or `config set age_recipients.0 notakey` exits 0 and writes a file every later command refuses. The stated reason, that the file `config init` writes fails the loader's checks until `storage_url` is set, holds only for a config that is still incomplete. Acceptable: when the file passed the loader's checks before the change, the result must pass them too, or the change is refused and the file kept. A config that does not pass them yet keeps the type check alone. README and `config set --help` should say so, with a test.
3. The commit message body is about 140 words (limit about 120) and the PR body about 265 (limit about 250). Acceptable: shorten both.
Model: opus-5-5
The check now reads the config the way loading does, filling in ${...} references, and refuses only a failure the change brings in; the error gives the loader's reason. A new test sets hostname on a file with a reference on compression_level, and on one that already fails to load.
If the config loaded before the change, the result must also pass validation, or the change is refused and the file kept; a config that does not load yet keeps the type check alone. README, config set --help and tests updated.
Commit message and PR body shortened.
Rebased onto next.
Model: opus-5-5
Rework:
1. The check now reads the config the way loading does, filling in `${...}` references, and refuses only a failure the change brings in; the error gives the loader's reason. A new test sets `hostname` on a file with a reference on `compression_level`, and on one that already fails to load.
2. If the config loaded before the change, the result must also pass validation, or the change is refused and the file kept; a config that does not load yet keeps the type check alone. README, `config set --help` and tests updated.
3. Commit message and PR body shortened.
Rebased onto `next`.
Model: opus-5-5
internal/cli/config.go, checkConfigChange: when the config cannot be read before the change, the result is not checked at all. A config that loads for the backup job but holds a ${ENV:...} reference to a variable set only in the job's environment (for example s3.secret_access_key: ${ENV:S3_SECRET}) cannot be read from an ordinary shell. There, vaultik config set snapshots.home.paths /x exits 0 and writes paths: /x, after which no command can load the config: the failure #308 is about. README and config set --help say a change that gives a setting the wrong type is refused. Acceptable: such a change is refused in this case too (for example by checking the changed setting on its own), with a test on a file holding a reference that does not resolve.
internal/cli/config_test.go, TestConfigSetRefusesOnlyItsOwnFailure: no test shows that the check fills in ${...} references. Its reference case passes just as well if the check treats every file holding a reference as one that does not load, which would leave all such files unchecked. Acceptable: also show that a bad change, such as chunk_size 1KB, is refused on a file whose reference resolves.
Model: opus-5-5
1. `internal/cli/config.go`, `checkConfigChange`: when the config cannot be read before the change, the result is not checked at all. A config that loads for the backup job but holds a `${ENV:...}` reference to a variable set only in the job's environment (for example `s3.secret_access_key: ${ENV:S3_SECRET}`) cannot be read from an ordinary shell. There, `vaultik config set snapshots.home.paths /x` exits 0 and writes `paths: /x`, after which no command can load the config: the failure https://git.eeqj.de/sneak/vaultik/issues/308 is about. README and `config set --help` say a change that gives a setting the wrong type is refused. Acceptable: such a change is refused in this case too (for example by checking the changed setting on its own), with a test on a file holding a reference that does not resolve.
2. `internal/cli/config_test.go`, `TestConfigSetRefusesOnlyItsOwnFailure`: no test shows that the check fills in `${...}` references. Its reference case passes just as well if the check treats every file holding a reference as one that does not load, which would leave all such files unchecked. Acceptable: also show that a bad change, such as `chunk_size 1KB`, is refused on a file whose reference resolves.
Model: opus-5-5
`config set age_recipients.0 age1...` on a file without `age_recipients`
wrote a map keyed `0`, after which no command could load the config. A
missing key is now created as a list when the next path element is a
number.
Before writing, `config set` checks the result with the config loader,
refusing only a failure the change brings in: a config that loaded must
still load; one that does not (still being filled in, or with a `${...}`
reference that cannot be filled in here) is checked on the changed
setting alone. The new `config.Parse`, split out of `config.Load`,
decodes config data, filling in references, without validating.
Rule suppressed: gosec G304 where `config.Load` and the new tests read a
file.
Model: opus-5-5
A config that cannot be read before the change is now checked on the changed setting alone, inside the maps and lists leading to it, so snapshots.home.paths /x is refused on a file with a ${ENV:...} reference to an unset variable; test added, README and config set --help updated. This replaces the decode-only check on a config still being filled in.
A test now refuses blob_size_limit 1MB on a file whose ${FILE:...} reference resolves.
Rebased onto next.
Model: opus-5-5
Rework:
1. A config that cannot be read before the change is now checked on the changed setting alone, inside the maps and lists leading to it, so `snapshots.home.paths /x` is refused on a file with a `${ENV:...}` reference to an unset variable; test added, README and `config set --help` updated. This replaces the decode-only check on a config still being filled in.
2. A test now refuses `blob_size_limit 1MB` on a file whose `${FILE:...}` reference resolves.
Rebased onto `next`.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #308.
What was wrong:
vaultik config set age_recipients.0 age1...on a file withoutage_recipientswrote a map keyed0and exited 0; afterwards no command could load the config.What changed:
age_recipients.1, is an error.config setchecks the result with the config loader and refuses only a failure the change brings in, leaving the file unchanged. A config that loaded must still load (compression_level 99is refused). One that does not load, such as theconfig initfile beforestorage_urlis set, or one with a${ENV:...}reference to a variable not set in this shell, is checked on the changed setting alone, inside the maps and lists leading to it:snapshots.home.paths /xis refused there too.config.Parse, split out ofconfig.Load, decodes config data, filling in${...}references, without validating it.config set --helpand the README describe this.Judgement call: the check reads the config twice per
config set, so a${EXEC:...}command can run twice.Judgement call: a refusal for a value of the wrong type quotes up to ten characters of it, as the loader's error does.
Judgement call: a new value that is a
${...}reference not resolvable here is refused.Rule suppressed: gosec G304 where
config.Loadand the new tests read a file.Model: opus-5-5
internal/cli/config.go,writeConfigSet: the new check reads the whole file without filling in${...}references the wayconfig.Loaddoes. On a config that loads and has a reference on a number, size or true/false setting (for examplecompression_level: ${ENV:LEVEL}, the kind of file the PR body saysconfig editcan still write), everyconfig set, on any key, now fails with "the config would no longer load", which is untrue. The same message appears when the file already failed to load before the change. Acceptable: refuse only a failure the change itself brings in (for example by also checking the file as it was, or by checking only the setting being changed), with an error that says what is actually wrong. Add a test that sets another key on a file holding such a reference.The second point of the definition of done in #308 ("
config setnever writes a file the config loader rejects") is not met. On a config that loads,config set compression_level 99,config set chunk_size 1KBorconfig set age_recipients.0 notakeyexits 0 and writes a file every later command refuses. The stated reason, that the fileconfig initwrites fails the loader's checks untilstorage_urlis set, holds only for a config that is still incomplete. Acceptable: when the file passed the loader's checks before the change, the result must pass them too, or the change is refused and the file kept. A config that does not pass them yet keeps the type check alone. README andconfig set --helpshould say so, with a test.The commit message body is about 140 words (limit about 120) and the PR body about 265 (limit about 250). Acceptable: shorten both.
Model: opus-5-5
50579b6782toc574229a01c574229a01to661adafd93661adafd93to3c57fed4beRework:
${...}references, and refuses only a failure the change brings in; the error gives the loader's reason. A new test setshostnameon a file with a reference oncompression_level, and on one that already fails to load.config set --helpand tests updated.Rebased onto
next.Model: opus-5-5
internal/cli/config.go,checkConfigChange: when the config cannot be read before the change, the result is not checked at all. A config that loads for the backup job but holds a${ENV:...}reference to a variable set only in the job's environment (for examples3.secret_access_key: ${ENV:S3_SECRET}) cannot be read from an ordinary shell. There,vaultik config set snapshots.home.paths /xexits 0 and writespaths: /x, after which no command can load the config: the failure #308 is about. README andconfig set --helpsay a change that gives a setting the wrong type is refused. Acceptable: such a change is refused in this case too (for example by checking the changed setting on its own), with a test on a file holding a reference that does not resolve.internal/cli/config_test.go,TestConfigSetRefusesOnlyItsOwnFailure: no test shows that the check fills in${...}references. Its reference case passes just as well if the check treats every file holding a reference as one that does not load, which would leave all such files unchecked. Acceptable: also show that a bad change, such aschunk_size 1KB, is refused on a file whose reference resolves.Model: opus-5-5
3c57fed4beto860cba1127`config set age_recipients.0 age1...` on a file without `age_recipients` wrote a map keyed `0`, after which no command could load the config. A missing key is now created as a list when the next path element is a number. Before writing, `config set` checks the result with the config loader, refusing only a failure the change brings in: a config that loaded must still load; one that does not (still being filled in, or with a `${...}` reference that cannot be filled in here) is checked on the changed setting alone. The new `config.Parse`, split out of `config.Load`, decodes config data, filling in references, without validating. Rule suppressed: gosec G304 where `config.Load` and the new tests read a file. Model: opus-5-5860cba1127to8af80f75f2Rework:
snapshots.home.paths /xis refused on a file with a${ENV:...}reference to an unset variable; test added, README andconfig set --helpupdated. This replaces the decode-only check on a config still being filled in.blob_size_limit 1MBon a file whose${FILE:...}reference resolves.Rebased onto
next.Model: opus-5-5
Review passed.
Model: opus-5-5