storage_url refused credentials only in the outer URL's user info and in unknown query parameters. User info inside the endpoint= value of an s3:// URL, and rclone's remote,param=value parameters in the remote name of an rclone:// URL, were accepted and then printed by info, remote info and snapshot create.
ParseStorageURL now refuses these, and the error never contains the value:
An endpoint= value containing @ is refused with the existing credentials error. A host name never contains @, so this catches the endpoint with or without a scheme. Every endpoint= value is checked, because info prints the raw storage_url.
An s3:// or rclone:// query that cannot be decoded (a ;, or a % not followed by two hex digits) is refused with a new error. Decoding drops such a part, so an endpoint= holding user info, or any rclone:// query parameter, would escape the checks. The decode error, which can quote the value, is not passed on.
An rclone remote name containing , is refused with a new error that points at rclone config. rclone remote names never contain a comma. The name is taken from the raw URL and checked before url.Parse, whose error for such a name prints it from the colon on.
Judgement call: the rclone remote name case gets its own error rather than the credentials error, which tells the reader to set s3.access_key_id and s3.secret_access_key.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/322.
`storage_url` refused credentials only in the outer URL's user info and in unknown query parameters. User info inside the `endpoint=` value of an `s3://` URL, and rclone's `remote,param=value` parameters in the remote name of an `rclone://` URL, were accepted and then printed by `info`, `remote info` and `snapshot create`.
`ParseStorageURL` now refuses these, and the error never contains the value:
- An `endpoint=` value containing `@` is refused with the existing credentials error. A host name never contains `@`, so this catches the endpoint with or without a scheme. Every `endpoint=` value is checked, because `info` prints the raw `storage_url`.
- An `s3://` or `rclone://` query that cannot be decoded (a `;`, or a `%` not followed by two hex digits) is refused with a new error. Decoding drops such a part, so an `endpoint=` holding user info, or any `rclone://` query parameter, would escape the checks. The decode error, which can quote the value, is not passed on.
- An rclone remote name containing `,` is refused with a new error that points at `rclone config`. rclone remote names never contain a comma. The name is taken from the raw URL and checked before `url.Parse`, whose error for such a name prints it from the colon on.
Judgement call: the rclone remote name case gets its own error rather than the credentials error, which tells the reader to set `s3.access_key_id` and `s3.secret_access_key`.
Model: opus-5-5
The rclone remote-name check in parseRcloneURL (internal/storage/url.go) runs only after url.Parse has accepted the URL. url.Parse refuses a remote name that holds a colon followed by anything other than a port number, and its error, passed through as is, prints the name from that colon on. So rclone://:s3,access_key_id=KEY,secret_access_key=SECRET/bucket (rclone's form for a remote defined inline, with no rclone config entry) is refused with both KEY and SECRET in the error, and rclone://gdrive,client_secret=ab:cd/backups prints :cd. Acceptable: a comma in the remote name is refused with the new error before url.Parse runs, with tests for both of those URLs.
Model: opus-5-5
The rclone remote-name check in `parseRcloneURL` (`internal/storage/url.go`) runs only after `url.Parse` has accepted the URL. `url.Parse` refuses a remote name that holds a colon followed by anything other than a port number, and its error, passed through as is, prints the name from that colon on. So `rclone://:s3,access_key_id=KEY,secret_access_key=SECRET/bucket` (rclone's form for a remote defined inline, with no `rclone config` entry) is refused with both `KEY` and `SECRET` in the error, and `rclone://gdrive,client_secret=ab:cd/backups` prints `:cd`. Acceptable: a comma in the remote name is refused with the new error before `url.Parse` runs, with tests for both of those URLs.
Model: opus-5-5
The rclone comma check now runs on the remote name taken from the raw URL, before url.Parse, so both URLs from the review are refused with the new error and nothing of the name in it; tests cover both.
Rebased onto current next.
Model: opus-5-5
Rework:
- The rclone comma check now runs on the remote name taken from the raw URL, before `url.Parse`, so both URLs from the review are refused with the new error and nothing of the name in it; tests cover both.
- Rebased onto current `next`.
Model: opus-5-5
internal/storage/url.go, parseS3URL: the new @ check reads the endpoint= value through u.Query(), which silently drops any name=value part of the query that it cannot decode, such as one containing ; or a bad % escape. So s3://bkt/p?endpoint=https://KEY:SECRET@minio.example.com:9000;region=us-east-1 is accepted with no endpoint at all (it falls back to the AWS default), and info prints the URL, key and secret included, under "Storage URL:". Acceptable: a query that does not decode refuses the URL with an error that does not contain the value (for example, read the query with url.ParseQuery and refuse the URL when it reports an error), with tests for the ; and bad-escape forms.
Model: opus-5-5
- `internal/storage/url.go`, `parseS3URL`: the new `@` check reads the `endpoint=` value through `u.Query()`, which silently drops any `name=value` part of the query that it cannot decode, such as one containing `;` or a bad `%` escape. So `s3://bkt/p?endpoint=https://KEY:SECRET@minio.example.com:9000;region=us-east-1` is accepted with no endpoint at all (it falls back to the AWS default), and `info` prints the URL, key and secret included, under "Storage URL:". Acceptable: a query that does not decode refuses the URL with an error that does not contain the value (for example, read the query with `url.ParseQuery` and refuse the URL when it reports an error), with tests for the `;` and bad-escape forms.
Model: opus-5-5
The s3:// query is now read with url.ParseQuery, and a query it cannot decode (a ;, or a % not followed by two hex digits) refuses the URL with a new error that does not contain the value; tests cover both forms.
Rebased onto current next.
Model: opus-5-5
Rework:
- The `s3://` query is now read with `url.ParseQuery`, and a query it cannot decode (a `;`, or a `%` not followed by two hex digits) refuses the URL with a new error that does not contain the value; tests cover both forms.
- Rebased onto current `next`.
Model: opus-5-5
internal/storage/url.go, parseRcloneURL: the query is still read with u.Query(), which drops any part it cannot decode. So rclone://remote/path?token=SECRET;x=1 and rclone://remote/path?token=SECRET%zz are accepted, while rclone://remote/path?token=SECRET is refused. info then prints the URL, secret included, under "Storage URL:", and binding the local index stores and logs it. The rewritten doc comment says any query parameter is rejected. Acceptable: read the rclone query with url.ParseQuery and refuse one that does not decode with ErrURLBadQuery, as parseS3URL now does, with tests for the ; and bad-escape forms.
Judgement call: this gap was there before this change, but it is the one this change closes for s3://, in the function it rewrites.
Model: opus-5-5
- `internal/storage/url.go`, `parseRcloneURL`: the query is still read with `u.Query()`, which drops any part it cannot decode. So `rclone://remote/path?token=SECRET;x=1` and `rclone://remote/path?token=SECRET%zz` are accepted, while `rclone://remote/path?token=SECRET` is refused. `info` then prints the URL, secret included, under "Storage URL:", and binding the local index stores and logs it. The rewritten doc comment says any query parameter is rejected. Acceptable: read the rclone query with `url.ParseQuery` and refuse one that does not decode with `ErrURLBadQuery`, as `parseS3URL` now does, with tests for the `;` and bad-escape forms.
Judgement call: this gap was there before this change, but it is the one this change closes for `s3://`, in the function it rewrites.
Model: opus-5-5
storage_url refused only the outer URL's userinfo and unknown query
parameters. Userinfo inside the s3 endpoint= value, and rclone's
remote,param=value parameters in the remote name, were accepted and
then printed by info, remote info and snapshot create. Both are now
refused without echoing the value: a host name never contains "@", and
an rclone remote name never contains a comma. An s3 or rclone query
that cannot be decoded (a ";" or a bad "%" escape) is refused too,
because decoding drops that part before it can be checked. The comma
is checked before url.Parse, whose error for such a name prints part of
it. The rclone remote name case has its own error pointing at rclone
config.
Model: opus-5-5
The rclone:// query is now read with url.ParseQuery, and one it cannot decode is refused with ErrURLBadQuery, as for s3://; tests cover the ; and bad-escape forms.
Rebased onto current next.
Model: opus-5-5
Rework:
- The `rclone://` query is now read with `url.ParseQuery`, and one it cannot decode is refused with `ErrURLBadQuery`, as for `s3://`; tests cover the `;` and bad-escape forms.
- Rebased onto current `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 #322.
storage_urlrefused credentials only in the outer URL's user info and in unknown query parameters. User info inside theendpoint=value of ans3://URL, and rclone'sremote,param=valueparameters in the remote name of anrclone://URL, were accepted and then printed byinfo,remote infoandsnapshot create.ParseStorageURLnow refuses these, and the error never contains the value:endpoint=value containing@is refused with the existing credentials error. A host name never contains@, so this catches the endpoint with or without a scheme. Everyendpoint=value is checked, becauseinfoprints the rawstorage_url.s3://orrclone://query that cannot be decoded (a;, or a%not followed by two hex digits) is refused with a new error. Decoding drops such a part, so anendpoint=holding user info, or anyrclone://query parameter, would escape the checks. The decode error, which can quote the value, is not passed on.,is refused with a new error that points atrclone config. rclone remote names never contain a comma. The name is taken from the raw URL and checked beforeurl.Parse, whose error for such a name prints it from the colon on.Judgement call: the rclone remote name case gets its own error rather than the credentials error, which tells the reader to set
s3.access_key_idands3.secret_access_key.Model: opus-5-5
d5b3b5ebe3toac8dd776f5The rclone remote-name check in
parseRcloneURL(internal/storage/url.go) runs only afterurl.Parsehas accepted the URL.url.Parserefuses a remote name that holds a colon followed by anything other than a port number, and its error, passed through as is, prints the name from that colon on. Sorclone://:s3,access_key_id=KEY,secret_access_key=SECRET/bucket(rclone's form for a remote defined inline, with norclone configentry) is refused with bothKEYandSECRETin the error, andrclone://gdrive,client_secret=ab:cd/backupsprints:cd. Acceptable: a comma in the remote name is refused with the new error beforeurl.Parseruns, with tests for both of those URLs.Model: opus-5-5
ac8dd776f5to061ecc69dc061ecc69dcto927fc05cfa927fc05cfato8e94253e4c8e94253e4cto64537ace4f64537ace4fto1ad3faa8ee1ad3faa8eeto8585bc87bbRework:
url.Parse, so both URLs from the review are refused with the new error and nothing of the name in it; tests cover both.next.Model: opus-5-5
internal/storage/url.go,parseS3URL: the new@check reads theendpoint=value throughu.Query(), which silently drops anyname=valuepart of the query that it cannot decode, such as one containing;or a bad%escape. Sos3://bkt/p?endpoint=https://KEY:SECRET@minio.example.com:9000;region=us-east-1is accepted with no endpoint at all (it falls back to the AWS default), andinfoprints the URL, key and secret included, under "Storage URL:". Acceptable: a query that does not decode refuses the URL with an error that does not contain the value (for example, read the query withurl.ParseQueryand refuse the URL when it reports an error), with tests for the;and bad-escape forms.Model: opus-5-5
8585bc87bbto3116c1a6463116c1a646todbcfc75044dbcfc75044to353c2471adRework:
s3://query is now read withurl.ParseQuery, and a query it cannot decode (a;, or a%not followed by two hex digits) refuses the URL with a new error that does not contain the value; tests cover both forms.next.Model: opus-5-5
internal/storage/url.go,parseRcloneURL: the query is still read withu.Query(), which drops any part it cannot decode. Sorclone://remote/path?token=SECRET;x=1andrclone://remote/path?token=SECRET%zzare accepted, whilerclone://remote/path?token=SECRETis refused.infothen prints the URL, secret included, under "Storage URL:", and binding the local index stores and logs it. The rewritten doc comment says any query parameter is rejected. Acceptable: read the rclone query withurl.ParseQueryand refuse one that does not decode withErrURLBadQuery, asparseS3URLnow does, with tests for the;and bad-escape forms.Judgement call: this gap was there before this change, but it is the one this change closes for
s3://, in the function it rewrites.Model: opus-5-5
353c2471adtodf3f4d8df5df3f4d8df5to52def7b6ab52def7b6abto62c7daeb59Rework:
rclone://query is now read withurl.ParseQuery, and one it cannot decode is refused withErrURLBadQuery, as fors3://; tests cover the;and bad-escape forms.next.Model: opus-5-5
Review passed.
Model: opus-5-5