config.Load warned "Config file has insecure permissions (contains S3 credentials)" for every config file its group or everyone can read, including a file:// config with no credentials. It now warns "Config file is readable by others and may contain S3 credentials" when s3.access_key_id or s3.secret_access_key is set, and "Config file is readable by others" otherwise. It says "may" because Load sees the credentials only after smartconfig has replaced any ${...} reference with its value, so a set credential need not be in the file. The path, mode and recommendation fields are unchanged.
snapshot purge against a destination store it could not list failed with syncing with remote: listing remote snapshots: listing remote snapshots: .... The listing error already carries that prefix, so syncWithRemote now returns it as it is, the way CleanupLocalSnapshots (used by prune) already did.
The config tests point os.Stderr at a file and rebuild the global logger around Load, because the logger writes to whatever os.Stderr was when it was initialized; that is why they do not run in parallel. The wording is chosen in a small Config method instead of inline because one more branch in Load goes over the linter's complexity limit.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/240.
- `config.Load` warned "Config file has insecure permissions (contains S3 credentials)" for every config file its group or everyone can read, including a `file://` config with no credentials. It now warns "Config file is readable by others and may contain S3 credentials" when `s3.access_key_id` or `s3.secret_access_key` is set, and "Config file is readable by others" otherwise. It says "may" because `Load` sees the credentials only after smartconfig has replaced any `${...}` reference with its value, so a set credential need not be in the file. The `path`, `mode` and `recommendation` fields are unchanged.
- `snapshot purge` against a destination store it could not list failed with `syncing with remote: listing remote snapshots: listing remote snapshots: ...`. The listing error already carries that prefix, so `syncWithRemote` now returns it as it is, the way `CleanupLocalSnapshots` (used by `prune`) already did.
The config tests point `os.Stderr` at a file and rebuild the global logger around `Load`, because the logger writes to whatever `os.Stderr` was when it was initialized; that is why they do not run in parallel. The wording is chosen in a small `Config` method instead of inline because one more branch in `Load` goes over the linter's complexity limit.
Model: opus-5-5
internal/config/config.go:416-421: the credentials wording is chosen from the values after smartconfig substitution. A config file whose s3.access_key_id and s3.secret_access_key are ${ENV:...} references (or any other substitution) is still warned about as "Config file contains S3 credentials and is readable by others", though the file holds no credential. That is the false claim #240 asks to remove ("says only what is true of the file"). Acceptable: the credentials wording appears only when the file itself holds a credential value rather than a ${...} reference, or the warning is worded so it is true either way; plus a test whose credentials come in through a reference.
Judgement call: this rejects the PR's disclosed reading that a substituted credential counts as set by the file.
Model: opus-5-5
1. `internal/config/config.go:416-421`: the credentials wording is chosen from the values after smartconfig substitution. A config file whose `s3.access_key_id` and `s3.secret_access_key` are `${ENV:...}` references (or any other substitution) is still warned about as "Config file contains S3 credentials and is readable by others", though the file holds no credential. That is the false claim https://git.eeqj.de/sneak/vaultik/issues/240 asks to remove ("says only what is true of the file"). Acceptable: the credentials wording appears only when the file itself holds a credential value rather than a `${...}` reference, or the warning is worded so it is true either way; plus a test whose credentials come in through a reference.
Judgement call: this rejects the PR's disclosed reading that a substituted credential counts as set by the file.
Model: opus-5-5
The warning for a config file that others can read always said the
file contained S3 credentials, so a file:// config with none got a
false claim. When s3.access_key_id or s3.secret_access_key is set it
now says the file may contain them, because Load sees the values only
after smartconfig has replaced any ${...} reference, so a set
credential need not be in the file. Otherwise it says the file is
readable by others.
snapshot purge wrapped the listing error, which already starts with
"listing remote snapshots:", in that prefix a second time.
syncWithRemote now returns it unwrapped, as CleanupLocalSnapshots
does.
Model: opus-5-5
The warning for a config file that sets S3 credentials now reads "Config file is readable by others and may contain S3 credentials", which is true whether the file holds the values or ${...} references. TestLoadWarnsReadableConfigWithS3Credentials now checks both, the second with ${ENV:...} references.
Model: opus-5-5
Rework delta:
1. The warning for a config file that sets S3 credentials now reads "Config file is readable by others and may contain S3 credentials", which is true whether the file holds the values or `${...}` references. `TestLoadWarnsReadableConfigWithS3Credentials` now checks both, the second with `${ENV:...}` references.
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 #240.
config.Loadwarned "Config file has insecure permissions (contains S3 credentials)" for every config file its group or everyone can read, including afile://config with no credentials. It now warns "Config file is readable by others and may contain S3 credentials" whens3.access_key_idors3.secret_access_keyis set, and "Config file is readable by others" otherwise. It says "may" becauseLoadsees the credentials only after smartconfig has replaced any${...}reference with its value, so a set credential need not be in the file. Thepath,modeandrecommendationfields are unchanged.snapshot purgeagainst a destination store it could not list failed withsyncing with remote: listing remote snapshots: listing remote snapshots: .... The listing error already carries that prefix, sosyncWithRemotenow returns it as it is, the wayCleanupLocalSnapshots(used byprune) already did.The config tests point
os.Stderrat a file and rebuild the global logger aroundLoad, because the logger writes to whateveros.Stderrwas when it was initialized; that is why they do not run in parallel. The wording is chosen in a smallConfigmethod instead of inline because one more branch inLoadgoes over the linter's complexity limit.Model: opus-5-5
internal/config/config.go:416-421: the credentials wording is chosen from the values after smartconfig substitution. A config file whoses3.access_key_idands3.secret_access_keyare${ENV:...}references (or any other substitution) is still warned about as "Config file contains S3 credentials and is readable by others", though the file holds no credential. That is the false claim #240 asks to remove ("says only what is true of the file"). Acceptable: the credentials wording appears only when the file itself holds a credential value rather than a${...}reference, or the warning is worded so it is true either way; plus a test whose credentials come in through a reference.Judgement call: this rejects the PR's disclosed reading that a substituted credential counts as set by the file.
Model: opus-5-5
cf034d1ac9toe5a13e8fdbThe warning for a config file that others can read always said the file contained S3 credentials, so a file:// config with none got a false claim. When s3.access_key_id or s3.secret_access_key is set it now says the file may contain them, because Load sees the values only after smartconfig has replaced any ${...} reference, so a set credential need not be in the file. Otherwise it says the file is readable by others. snapshot purge wrapped the listing error, which already starts with "listing remote snapshots:", in that prefix a second time. syncWithRemote now returns it unwrapped, as CleanupLocalSnapshots does. Model: opus-5-5e5a13e8fdbtob130c0ceb2Rework delta:
${...}references.TestLoadWarnsReadableConfigWithS3Credentialsnow checks both, the second with${ENV:...}references.Model: opus-5-5
Review passed.
Model: opus-5-5