Stop config set echoing secrets; reject credential-bearing storage URLs #184

Merged
clawbot merged 1 commits from issue-166-config-set-secret-hardening into next 2026-09-22 12:28:32 +02:00
Collaborator

Closes #166 (security hardening, severity low).

Three changes:

  • config set prints only the key name after a write, never the value. A value can be a secret such as s3.secret_access_key, and echoing it leaks into captured stdout and pasted terminals. The write logic moves into a new writeConfigSet helper so the no-echo behaviour is testable.
  • config set tightens a pre-existing group- or world-readable config to 0600 after writing. os.WriteFile does not change an existing file's mode, so the old stat-and-preserve-mode block was dead code; it is removed and replaced with an os.Chmod when the loose bits are set.
  • ParseStorageURL rejects s3:// and rclone:// URLs whose userinfo or query carries credentials, naming s3.access_key_id and s3.secret_access_key as where they belong. rclone:// accepts no query parameters, so a misspelt one is now caught instead of silently sending the backup to the default AWS endpoint. On a url.Parse failure only the inner cause is wrapped, so the raw (possibly credential-bearing) URL is not echoed. file:// is untouched.

Trap worth noting for review: the rejection message intentionally names the S3 credential keys even for a plain misspelt parameter, matching the issue's Acceptable section.

Judgement call (disclosed): new URL tests were added to the existing internal/storage/url_parse_test.go rather than a separate url_test.go, to avoid two test files for one source file.

make check is green.

Model: opus-4-8

Closes https://git.eeqj.de/sneak/vaultik/issues/166 (security hardening, severity low). Three changes: - `config set` prints only the key name after a write, never the value. A value can be a secret such as `s3.secret_access_key`, and echoing it leaks into captured stdout and pasted terminals. The write logic moves into a new `writeConfigSet` helper so the no-echo behaviour is testable. - `config set` tightens a pre-existing group- or world-readable config to `0600` after writing. `os.WriteFile` does not change an existing file's mode, so the old stat-and-preserve-mode block was dead code; it is removed and replaced with an `os.Chmod` when the loose bits are set. - `ParseStorageURL` rejects `s3://` and `rclone://` URLs whose userinfo or query carries credentials, naming `s3.access_key_id` and `s3.secret_access_key` as where they belong. `rclone://` accepts no query parameters, so a misspelt one is now caught instead of silently sending the backup to the default AWS endpoint. On a `url.Parse` failure only the inner cause is wrapped, so the raw (possibly credential-bearing) URL is not echoed. `file://` is untouched. Trap worth noting for review: the rejection message intentionally names the S3 credential keys even for a plain misspelt parameter, matching the issue's Acceptable section. Judgement call (disclosed): new URL tests were added to the existing `internal/storage/url_parse_test.go` rather than a separate `url_test.go`, to avoid two test files for one source file. `make check` is green. Model: opus-4-8
clawbot added 1 commit 2026-09-22 12:02:49 +02:00
config set now prints only the key name after a write, never the value:
a value may be a secret such as s3.secret_access_key, and echoing it
leaks into captured stdout and pasted terminals. The set logic moves
into writeConfigSet so this is testable.

config set also tightens a pre-existing group- or world-readable config
to 0600 after writing. os.WriteFile does not change an existing file's
mode, so the previous stat-and-preserve-mode block had no effect; it is
removed.

ParseStorageURL now rejects s3:// and rclone:// URLs that carry
credentials in the userinfo or an unknown query parameter, and names
s3.access_key_id and s3.secret_access_key as where credentials belong;
rclone:// accepts no parameters, so a misspelt one is caught rather than
silently sending the backup to the default endpoint. On a url.Parse
failure only the inner cause is wrapped, so the raw URL is not echoed.
file:// is unchanged.

Model: opus-4-8
clawbot added the needs-review label 2026-09-22 12:02:57 +02:00
clawbot self-assigned this 2026-09-22 12:02:57 +02:00
Author
Collaborator

PASS

Model: opus-4-8

PASS Model: opus-4-8
clawbot merged commit 39aef1c47c into next 2026-09-22 12:28:32 +02:00
clawbot deleted branch issue-166-config-set-secret-hardening 2026-09-22 12:28:32 +02:00
Sign in to join this conversation.