vpm: refuse world-readable signing keys, never assert SBOM license conclusions (#30626)

* vpm: validate registry TLS, check returned identity, skip client setup for serve

Three review findings on the registry transport, each with a test.

Registry requests over HTTPS now validate certificates, whether or not
they carry a token; tokenless metadata was previously fetched with
validation off, so a MITM could replace it. Plain HTTP and non-registry
traffic are unchanged.

decode_registry_module now checks the body against the requested route: a
body naming another module, or another version when one was requested, is
refused instead of decoded. A cross-wired registry previously flowed into
resolution output.

v registry serve no longer requires a writable VModules home. Dispatch runs
before ensure_vmodules_dir_exist, and init_settings skips the log-cache
setup for the registry command, so a service account can serve a registry
from its working directory.

* vpm: refuse world-readable signing keys, never assert SBOM license conclusions

Two Codex review findings on the registry, each with a test.

A signing key file readable by group or others lets any local user forge
signatures, so it is no longer loaded. The check is Unix-only by design:
Windows has no equivalent mode check and accepts the file there, while a
missing file is refused everywhere. A refused key serves unsigned, exactly
like a missing one, which verification already fails closed on.

`licenseConcluded` is a derived result and nothing here derives one, so it
is now always NOASSERTION. The publisher's declaration stays in
`licenseDeclared`. Consumers previously read an unexamined claim as a
compliance result.
This commit is contained in:
Mahdi ramezaan zaade authored and GitHub committed 2026-10-18 13:03:15 +03:00
1 parent a93b2f1c32
commit 182ebde118
12 files changed
+153 -13

No files matched your search

+8
View File
@@ -27,6 +27,14 @@ fn test_unqualified_vpm_token_covers_one_private_registry() {
assert registry_token('https://vpm.example.com/a_module') == 'tok-all'
}
fn test_registry_transport_validates_https_without_a_token() {
assert registry_validates_tls(true, 'https://vpm.example.com/module')
assert registry_validates_tls(true, 'HTTPS://vpm.example.com/module')
assert !registry_validates_tls(true, 'http://vpm.example.com/module')
assert !registry_validates_tls(false, 'https://vpm.example.com/module')
assert !registry_validates_tls(true, '::::not a url::::')
}
// A scoped token must win over the unqualified one, because a machine holding
// tokens for several private registries must not send one registry's token to
// another.
+13
View File
@@ -100,6 +100,18 @@ fn registry_http_get(url string) !http.Response {
return vpm_http_request(.get, url, '', true)
}
// registry_validates_tls reports whether a registry request should validate
// TLS certificates. Registry records are security-relevant whether or not a
// token is configured, so HTTPS requests validate them; HTTP and ordinary VPM
// requests keep the transport's existing behaviour.
fn registry_validates_tls(registry_request bool, url string) bool {
if !registry_request {
return false
}
parsed := urllib.parse(url) or { return false }
return parsed.scheme.to_lower() == 'https'
}
fn vpm_http_request(method http.Method, url string, data string, authenticate bool) !http.Response {
mut header := http.Header{}
token := if authenticate { registry_token(url) } else { '' }
@@ -111,6 +123,7 @@ fn vpm_http_request(method http.Method, url string, data string, authenticate bo
url: url
data: data
header: header
validate: registry_validates_tls(authenticate, url)
read_timeout: vpm_http_timeout
write_timeout: vpm_http_timeout
max_retries: 1
+20 -1
View File
@@ -550,10 +550,29 @@ pub fn handle_request(registry Registry, method string, path string, query map[s
return not_found_body
}
// registry_key_is_private reports whether the key file may be loaded. On Unix
// a key readable by group or others lets any local user forge signatures, so
// it is refused. Windows has no equivalent mode check, so the file is
// accepted there.
fn registry_key_is_private(path string) bool {
$if windows {
return true
}
st := os.stat(path) or { return false }
mode := st.get_mode()
return !mode.group.read && !mode.others.read
}
// signing_key loads the registry's private key seed, from `VPM_REGISTRY_KEY`
// when set and from the key file beside the index otherwise. Both hold hex.
fn (r &Registry) signing_key() ?ed25519.PrivateKey {
raw := os.getenv_opt(registry_key_env) or { os.read_file(registry_key_path()) or { return none } }
raw := os.getenv_opt(registry_key_env) or {
path := registry_key_path()
if !registry_key_is_private(path) {
return none
}
os.read_file(path) or { return none }
}
hex := raw.trim_space()
if hex == '' {
return none
+16 -7
View File
@@ -59,17 +59,26 @@ fn registry_get(url string, path string) !string {
return resp.body
}
// decode_registry_module reads one module body. A body that is valid JSON but
// names no module is refused, because a registry that reports not-found in the
// body rather than in the status line produces exactly that, and decoding it
// would hand back a module with an empty name.
fn decode_registry_module(url string, name string, body string) !RegistryModule {
// decode_registry_module reads one module body and checks it against the
// requested route. A body that is valid JSON but names no module is refused,
// because a registry that reports not-found in the body rather than in the
// status line produces exactly that, and decoding it would hand back a module
// with an empty name. A body naming another module or, when a version was
// requested, another version is refused for the same reason: the caller must
// be able to trust that the response belongs to the requested route.
fn decode_registry_module(url string, name string, version string, body string) !RegistryModule {
info := json2.decode[RegistryModule](body) or {
return error('the registry at `${url}` did not describe `${name}` as JSON: ${err.msg()}')
}
if info.name == '' {
return error('the registry at `${url}` holds no metadata for `${name}`.')
}
if info.name != name {
return error('the registry at `${url}` answered for `${name}` with metadata for `${info.name}`.')
}
if version != '' && info.version != version {
return error('the registry at `${url}` answered `${info.name}@${info.version}` for `${name}@${version}`.')
}
return info
}
@@ -101,7 +110,7 @@ pub fn fetch_registry_versions(url string, name string) ![]string {
// returned as a zero-valued RegistryModule, because a caller resolving a module
// has to be able to go on to the next candidate registry.
pub fn fetch_registry_latest(url string, name string) !RegistryModule {
return decode_registry_module(url, name, registry_get(url, '/${name}/@latest')!)
return decode_registry_module(url, name, '', registry_get(url, '/${name}/@latest')!)
}
// fetch_registry_info returns the metadata for `name` at `version`, as served by
@@ -112,7 +121,7 @@ pub fn fetch_registry_latest(url string, name string) !RegistryModule {
// version. It is never returned as a zero-valued RegistryModule, so a caller can
// tell a version that was never published from one whose metadata is empty.
pub fn fetch_registry_info(url string, name string, version string) !RegistryModule {
return decode_registry_module(url, name, registry_get(url, '/${name}/@v/${version}.info')!)
return decode_registry_module(url, name, version, registry_get(url, '/${name}/@v/${version}.info')!)
}
// fetch_registry_config returns the configuration document the registry at `url`
+15 -3
View File
@@ -79,7 +79,7 @@ fn test_the_info_body_is_the_one_the_server_encodes() {
}
fn test_the_info_body_decodes_field_by_field() {
info := decode_registry_module(client_registry_url, 'alpha', client_info_body) or {
info := decode_registry_module(client_registry_url, 'alpha', '1.0.0', client_info_body) or {
assert false, 'the info body did not decode: ${err.msg()}'
return
}
@@ -94,6 +94,18 @@ fn test_the_info_body_decodes_field_by_field() {
assert info.yanked == false
}
fn test_registry_metadata_for_another_module_is_rejected() {
wrong_name_body := client_info_body.replace('"name":"alpha"', '"name":"beta"')
decode_registry_module(client_registry_url, 'alpha', '', wrong_name_body) or { return }
assert false, 'metadata for another module was accepted for the requested route'
}
fn test_registry_metadata_for_another_version_is_rejected() {
wrong_version_body := client_info_body.replace('"version":"1.0.0"', '"version":"2.0.0"')
decode_registry_module(client_registry_url, 'alpha', '1.0.0', wrong_version_body) or { return }
assert false, 'metadata for another version was accepted for the requested route'
}
// The status line is the contract, and a 404 is the only failure status the
// router can produce.
fn test_a_404_answer_is_absent() {
@@ -115,7 +127,7 @@ fn test_a_not_found_body_is_noticed_whatever_the_status() {
// What the fetches do with an absent answer: an error, so a caller can go on to
// the next candidate registry, rather than a module that installs nothing.
fn test_the_not_found_body_is_an_error_not_an_empty_module() {
decode_registry_module(client_registry_url, 'alpha', not_found_body) or { return }
decode_registry_module(client_registry_url, 'alpha', '', not_found_body) or { return }
assert false, 'the not-found body was decoded as a module'
}
@@ -127,7 +139,7 @@ fn test_the_not_found_body_is_not_an_empty_version_list() {
// An answer that is not JSON at all is a proxy's page or a truncated body, and is
// reported as such rather than as a module.
fn test_a_body_that_is_not_a_module_is_an_error() {
decode_registry_module(client_registry_url, 'alpha', '<html>502 Bad Gateway</html>') or { return }
decode_registry_module(client_registry_url, 'alpha', '', '<html>502 Bad Gateway</html>') or { return }
assert false, 'an html error page was decoded as a module'
}
+41
View File
@@ -154,3 +154,44 @@ fn test_the_config_route_names_the_vouching_key() {
result := handle_request(r, 'GET', '/config.json', map[string]string{})
assert result.contains(vouching_public.hex()), result
}
// A key file readable by group or others lets any local user forge
// signatures, so it must not be loaded. Unix-only: Windows has no equivalent
// mode check and accepts the file there.
fn test_group_readable_signing_key_is_not_loaded() {
$if windows {
return
}
os.unsetenv('VPM_REGISTRY_KEY')
original := os.getwd()
root := os.join_path(os.vtmp_dir(), 'vpm_signing_key_perm')
os.mkdir_all(os.join_path(root, '.vpm-registry')) or { panic(err) }
os.chdir(root) or { panic(err) }
defer {
os.chdir(original) or {}
os.rmdir_all(root) or {}
}
public, private := ed25519.generate_key() or {
assert false, 'key generation failed: ${err}'
return
}
key_file := os.join_path('.vpm-registry', 'signing.key')
os.write_file(key_file, private.seed().hex()) or { panic(err) }
r := new_registry()
os.chmod(key_file, 0o644) or { panic(err) }
assert r.public_key_hex() == '', 'a group-readable signing key was loaded'
os.chmod(key_file, 0o600) or { panic(err) }
assert r.public_key_hex() == public.hex(), 'a private signing key was refused'
}
// The permission gate above is Unix-only by design: Windows has no equivalent
// mode check, so the loader accepts the file there, while a missing file is
// refused everywhere. This pins that split on every platform, including the
// ones where the test above returns immediately.
fn test_key_file_gate_is_platform_split() {
$if windows {
assert registry_key_is_private('C:\\nonexistent-signing.key')
} $else {
assert !registry_key_is_private('/nonexistent-signing-key')
}
}
+4 -1
View File
@@ -117,7 +117,10 @@ pub fn (r &Registry) spdx_document(namespace string, download_base string) SpdxD
spdxid: spdx_id_for(v.name, v.version)
version_info: v.version
download_location: '${artifact_base}/${v.name}/@v/${v.version}.zip'
license_concluded: if v.license == '' { 'NOASSERTION' } else { v.license }
// A conclusion is a derived result, and nothing here derives one:
// the publisher's declaration is recorded as declared, while
// concluded stays NOASSERTION even when a license is present.
license_concluded: 'NOASSERTION'
license_declared: if v.license == '' { 'NOASSERTION' } else { v.license }
copyright_text: 'NOASSERTION'
checksums: checksums
+15
View File
@@ -181,3 +181,18 @@ fn test_spdx_json_uses_the_standard_field_names() {
}
}
}
// A conclusion is a derived result, and nothing here derives one: the
// publisher's declaration must be recorded as declared while concluded stays
// NOASSERTION, or consumers read an unexamined claim as a compliance result.
fn test_concluded_license_is_never_asserted() {
r := new_sbom_demo_registry()
doc := r.spdx_document(sbom_namespace, sbom_download_base)
assert doc.packages.len == 2
for pkg in doc.packages {
assert pkg.license_concluded == 'NOASSERTION', 'concluded license for ${pkg.name}: ${pkg.license_concluded}'
if pkg.name == 'serde' {
assert pkg.license_declared == 'MIT', pkg.license_declared
}
}
}
+2 -1
View File
@@ -74,13 +74,14 @@ fn init_settings() VpmSettings {
is_no_inc := os.getenv('VPM_NO_INCREMENT') != ''
is_dbg := os.getenv('VPM_DEBUG') != ''
is_ci := os.getenv('CI') != ''
skip_client_home_setup := is_registry_command(args)
mut logger := &log.Log{}
logger.set_output_stream(os.stderr())
if is_dbg {
logger.set_level(.debug)
}
if !is_ci && !is_dbg {
if !is_ci && !is_dbg && !skip_client_home_setup {
// Log by default, but only in the global location, no matter if --local was passed:
cache_path := os.join_path(global_vmodules_path, '.cache')
os.mkdir_all(cache_path, mode: 0o700) or { panic(err) }
+7
View File
@@ -17,6 +17,13 @@ fn test_get_mirror_urls_from_args_supports_short_and_long_flags() {
assert mirror_urls == ['https://mirror1.example', 'https://mirror2.example']
}
fn test_registry_command_skips_client_home_setup() {
assert is_registry_command(['registry', 'serve'])
assert is_registry_command(['--local', 'registry', 'serve'])
assert !is_registry_command(['install', 'registry'])
assert !is_registry_command(['install', '-m', 'https://mirror.example', 'registry'])
}
fn test_build_install_server_urls_prioritizes_default_servers() {
server_urls := build_install_server_urls(['https://official1.example', 'https://official2.example'], [
'https://mirror1.example',
+11
View File
@@ -43,6 +43,10 @@ fn main() {
}
mut query := parse_query_args(args, vpm_command)
vpm_log(@FILE_LINE, @FN, 'query: ${query}')
if is_registry_command(args) {
vpm_registry(query)
return
}
ensure_vmodules_dir_exist()
match vpm_command {
'help' {
@@ -117,6 +121,13 @@ fn parse_vpm_command(args []string) string {
return ''
}
// is_registry_command reports whether the requested command is the registry
// server. It is checked before client-only home and cache setup, so serving a
// registry does not require a writable VModules directory.
fn is_registry_command(args []string) bool {
return parse_vpm_command(args) == 'registry'
}
fn parse_query_args(args []string, vpm_command string) []string {
mut query := []string{}
mut skip_next := false
+1
View File
@@ -158,6 +158,7 @@ its own registry requests. The registry server in this repository reads no crede
authenticates nothing.
- `Authorization: Bearer <token>` is set by the VPM client only.
- Registry requests over HTTPS validate TLS certificates, whether or not they carry a token.
- The token comes from `VPM_TOKEN_<HOST>`, where `<HOST>` is the registry's hostname with `.` and
`-` folded to `_` and uppercased, so `https://vpm.example.com/` reads
`VPM_TOKEN_VPM_EXAMPLE_COM`. `VPM_TOKEN` is the fallback when no scoped variable is set, and