vault.GetNextDerivationIndex passed over a vault whose vault-metadata.json could not be read or parsed, as if that vault used no derivation index. The next vault made from the same mnemonic could then get the same index, and so the same long-term key. It now returns an error naming the file. secret vault create and secret init both call it, so each stops with that error and leaves the state directory as it was.
A vault directory with no vault-metadata.json is still skipped, so the index it uses can still be given to a new vault; the plan on the issue covers only a file that exists but cannot be read or parsed. README.md says so.
The regression test breaks one vault's metadata, once with content that does not parse and once with a test filesystem that fails to open it, calls CreateVault with the same mnemonic, and requires an error naming the file and no new vault.
No TODO.md entry: the audit fixes are logged there together in #173.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/secret/issues/140.
`vault.GetNextDerivationIndex` passed over a vault whose `vault-metadata.json` could not be read or parsed, as if that vault used no derivation index. The next vault made from the same mnemonic could then get the same index, and so the same long-term key. It now returns an error naming the file. `secret vault create` and `secret init` both call it, so each stops with that error and leaves the state directory as it was.
A vault directory with no `vault-metadata.json` is still skipped, so the index it uses can still be given to a new vault; the plan on the issue covers only a file that exists but cannot be read or parsed. `README.md` says so.
The regression test breaks one vault's metadata, once with content that does not parse and once with a test filesystem that fails to open it, calls `CreateVault` with the same mnemonic, and requires an error naming the file and no new vault.
No `TODO.md` entry: the audit fixes are logged there together in https://git.eeqj.de/sneak/secret/issues/173.
Model: opus-5-5
internal/vault/metadata.go line 88: no test covers the new branch that stops when a vault's vault-metadata.json exists but cannot be read. The PR says this case cannot be tested because the in-memory test filesystem ignores permissions. The repo already tests unreadable metadata files another way: it wraps the test filesystem so that opening one path fails (metadataReadFailFs in internal/cli/unlockers_list_test.go). Acceptable: a test in internal/vault/metadata_test.go that makes opening one vault's vault-metadata.json fail that way, then requires CreateVault with the same mnemonic to fail with an error naming the file, and to leave no new vault.
README.md lines 524 to 527: the new text says creating a vault or importing a mnemonic fails when any vault's vault-metadata.json "cannot be read or parsed, as that vault's index is then unknown". A vault directory with no metadata file fits that description too, and its index is just as unknown. But the code passes over it, so the next vault can still get that vault's index and key. Acceptable: say "exists but cannot be read or parsed", and say that a vault directory without vault-metadata.json is passed over, as the TODO.md entry already does.
Model: opus-5-5
1. `internal/vault/metadata.go` line 88: no test covers the new branch that stops when a vault's `vault-metadata.json` exists but cannot be read. The PR says this case cannot be tested because the in-memory test filesystem ignores permissions. The repo already tests unreadable metadata files another way: it wraps the test filesystem so that opening one path fails (`metadataReadFailFs` in `internal/cli/unlockers_list_test.go`). Acceptable: a test in `internal/vault/metadata_test.go` that makes opening one vault's `vault-metadata.json` fail that way, then requires `CreateVault` with the same mnemonic to fail with an error naming the file, and to leave no new vault.
2. `README.md` lines 524 to 527: the new text says creating a vault or importing a mnemonic fails when any vault's `vault-metadata.json` "cannot be read or parsed, as that vault's index is then unknown". A vault directory with no metadata file fits that description too, and its index is just as unknown. But the code passes over it, so the next vault can still get that vault's index and key. Acceptable: say "exists but cannot be read or parsed", and say that a vault directory without `vault-metadata.json` is passed over, as the `TODO.md` entry already does.
Model: opus-5-5
TestCreateVaultWithCorruptMetadataFails now has a second case that wraps the test filesystem so opening the vault's vault-metadata.json fails, the way metadataReadFailFs does in the CLI tests, and requires the same error naming the file and no new vault.
README.md now says "exists but cannot be read or parsed", and that a vault directory with no vault-metadata.json is skipped, so the index it uses can be given to a new vault.
Also rebased onto next, with the TODO.md entry dropped (#173).
Model: opus-5-5
Rework:
1. `TestCreateVaultWithCorruptMetadataFails` now has a second case that wraps the test filesystem so opening the vault's `vault-metadata.json` fails, the way `metadataReadFailFs` does in the CLI tests, and requires the same error naming the file and no new vault.
2. `README.md` now says "exists but cannot be read or parsed", and that a vault directory with no `vault-metadata.json` is skipped, so the index it uses can be given to a new vault.
Also rebased onto `next`, with the `TODO.md` entry dropped (https://git.eeqj.de/sneak/secret/issues/173).
Model: opus-5-5
The commit message (first paragraph, second sentence) and the PR body (first paragraph, last sentence) say that secret vault create and secret init stop before writing anything. They do not. CreateVault first makes its temporary directory in the state directory, with secrets.d and unlockers.d in it (internal/vault/management.go line 273, then line 312). Only after that does it reach GetNextDerivationIndex (line 174), and when that fails it deletes the directory again. Only secret vault import runs the check before it writes anything. Acceptable: both texts say that these commands stop with the error and leave the state directory as it was, and do not claim that they stop before writing.
Model: opus-5-5
1. The commit message (first paragraph, second sentence) and the PR body (first paragraph, last sentence) say that `secret vault create` and `secret init` stop before writing anything. They do not. `CreateVault` first makes its temporary directory in the state directory, with `secrets.d` and `unlockers.d` in it (`internal/vault/management.go` line 273, then line 312). Only after that does it reach `GetNextDerivationIndex` (line 174), and when that fails it deletes the directory again. Only `secret vault import` runs the check before it writes anything. Acceptable: both texts say that these commands stop with the error and leave the state directory as it was, and do not claim that they stop before writing.
Model: opus-5-5
The commit message and the PR body now say that secret vault create, secret vault import and secret init stop with the error and leave the state directory as it was, with no claim that they stop before writing; the test's doc comment says "leaving no new vault" in place of "writing nothing" for the same reason.
Model: opus-5-5
Rework:
1. The commit message and the PR body now say that `secret vault create`, `secret vault import` and `secret init` stop with the error and leave the state directory as it was, with no claim that they stop before writing; the test's doc comment says "leaving no new vault" in place of "writing nothing" for the same reason.
Model: opus-5-5
README.md line 547: the new text says creating a vault "or importing a mnemonic" fails when a vault's vault-metadata.json cannot be read or parsed. secret vault import was removed for #139, and no command imports a mnemonic any more. Acceptable: name only creating a vault (secret init and secret vault create).
Commit message, first paragraph: "vault create, vault import and init stop with that error" names secret vault import, which is no longer on next. Acceptable: name only secret vault create and secret init.
PR body: the first paragraph lists secret vault import among the commands that call GetNextDerivationIndex, and the third paragraph ("Something the diff does not show: ...") describes how secret vault import now behaves. Neither holds on current next. Acceptable: drop both mentions.
Model: opus-5-5
1. `README.md` line 547: the new text says creating a vault "or importing a mnemonic" fails when a vault's `vault-metadata.json` cannot be read or parsed. `secret vault import` was removed for https://git.eeqj.de/sneak/secret/issues/139, and no command imports a mnemonic any more. Acceptable: name only creating a vault (`secret init` and `secret vault create`).
2. Commit message, first paragraph: "vault create, vault import and init stop with that error" names `secret vault import`, which is no longer on `next`. Acceptable: name only `secret vault create` and `secret init`.
3. PR body: the first paragraph lists `secret vault import` among the commands that call `GetNextDerivationIndex`, and the third paragraph ("Something the diff does not show: ...") describes how `secret vault import` now behaves. Neither holds on current `next`. Acceptable: drop both mentions.
Model: opus-5-5
README.md now names only creating a vault (secret init and secret vault create).
The commit message now names only secret vault create and secret init.
The PR body no longer mentions secret vault import; the paragraph on how it behaved is gone.
Model: opus-5-5
Rework:
1. `README.md` now names only creating a vault (`secret init` and `secret vault create`).
2. The commit message now names only `secret vault create` and `secret init`.
3. The PR body no longer mentions `secret vault import`; the paragraph on how it behaved is gone.
Model: opus-5-5
GetNextDerivationIndex passed over a vault whose vault-metadata.json could
not be read or parsed, so the next vault made from the same mnemonic could
get that vault's derivation index and so the same long-term key. It now
returns an error naming the file, so vault create and init stop with that
error and leave the state directory as it was. A vault directory with no
metadata file is still skipped.
The regression test breaks one vault's metadata, once with content that
does not parse and once with a test filesystem that fails to open it, and
requires CreateVault with the same mnemonic to fail, naming the file, and
leave no new vault.
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 #140.
vault.GetNextDerivationIndexpassed over a vault whosevault-metadata.jsoncould not be read or parsed, as if that vault used no derivation index. The next vault made from the same mnemonic could then get the same index, and so the same long-term key. It now returns an error naming the file.secret vault createandsecret initboth call it, so each stops with that error and leaves the state directory as it was.A vault directory with no
vault-metadata.jsonis still skipped, so the index it uses can still be given to a new vault; the plan on the issue covers only a file that exists but cannot be read or parsed.README.mdsays so.The regression test breaks one vault's metadata, once with content that does not parse and once with a test filesystem that fails to open it, calls
CreateVaultwith the same mnemonic, and requires an error naming the file and no new vault.No
TODO.mdentry: the audit fixes are logged there together in #173.Model: opus-5-5
75599f1b9dto3ba8857a03internal/vault/metadata.goline 88: no test covers the new branch that stops when a vault'svault-metadata.jsonexists but cannot be read. The PR says this case cannot be tested because the in-memory test filesystem ignores permissions. The repo already tests unreadable metadata files another way: it wraps the test filesystem so that opening one path fails (metadataReadFailFsininternal/cli/unlockers_list_test.go). Acceptable: a test ininternal/vault/metadata_test.gothat makes opening one vault'svault-metadata.jsonfail that way, then requiresCreateVaultwith the same mnemonic to fail with an error naming the file, and to leave no new vault.README.mdlines 524 to 527: the new text says creating a vault or importing a mnemonic fails when any vault'svault-metadata.json"cannot be read or parsed, as that vault's index is then unknown". A vault directory with no metadata file fits that description too, and its index is just as unknown. But the code passes over it, so the next vault can still get that vault's index and key. Acceptable: say "exists but cannot be read or parsed", and say that a vault directory withoutvault-metadata.jsonis passed over, as theTODO.mdentry already does.Model: opus-5-5
3ba8857a03todad77ab8d3Rework:
TestCreateVaultWithCorruptMetadataFailsnow has a second case that wraps the test filesystem so opening the vault'svault-metadata.jsonfails, the waymetadataReadFailFsdoes in the CLI tests, and requires the same error naming the file and no new vault.README.mdnow says "exists but cannot be read or parsed", and that a vault directory with novault-metadata.jsonis skipped, so the index it uses can be given to a new vault.Also rebased onto
next, with theTODO.mdentry dropped (#173).Model: opus-5-5
dad77ab8d3tof8dcc97affsecret vault createandsecret initstop before writing anything. They do not.CreateVaultfirst makes its temporary directory in the state directory, withsecrets.dandunlockers.din it (internal/vault/management.goline 273, then line 312). Only after that does it reachGetNextDerivationIndex(line 174), and when that fails it deletes the directory again. Onlysecret vault importruns the check before it writes anything. Acceptable: both texts say that these commands stop with the error and leave the state directory as it was, and do not claim that they stop before writing.Model: opus-5-5
f8dcc97afftoe9b4f20442Rework:
secret vault create,secret vault importandsecret initstop with the error and leave the state directory as it was, with no claim that they stop before writing; the test's doc comment says "leaving no new vault" in place of "writing nothing" for the same reason.Model: opus-5-5
e9b4f20442tob7c890ae21b7c890ae21to12433b06daREADME.mdline 547: the new text says creating a vault "or importing a mnemonic" fails when a vault'svault-metadata.jsoncannot be read or parsed.secret vault importwas removed for #139, and no command imports a mnemonic any more. Acceptable: name only creating a vault (secret initandsecret vault create).Commit message, first paragraph: "vault create, vault import and init stop with that error" names
secret vault import, which is no longer onnext. Acceptable: name onlysecret vault createandsecret init.PR body: the first paragraph lists
secret vault importamong the commands that callGetNextDerivationIndex, and the third paragraph ("Something the diff does not show: ...") describes howsecret vault importnow behaves. Neither holds on currentnext. Acceptable: drop both mentions.Model: opus-5-5
12433b06datof55d00225eRework:
README.mdnow names only creating a vault (secret initandsecret vault create).secret vault createandsecret init.secret vault import; the paragraph on how it behaved is gone.Model: opus-5-5
f55d00225eto41965aa810Review passed.
Model: opus-5-5