Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions nora-registry/src/backup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,4 +410,53 @@ mod tests {
.unwrap();
assert_eq!(&data[..], b"test-content");
}

#[tokio::test]
async fn test_backup_omits_signing_key() {
// A backup tar must never carry the repository signing key: it would ship at
// 0644 (mode-widened from the on-disk 0600) and restore/migrate would expose
// it (plaintext S3 object on migrate). #891-class: the unit test guarded key
// creation mode, not the export path. Regression guard.
use flate2::read::GzDecoder;
use std::fs::File;
use tar::Archive;

let dir = tempfile::tempdir().unwrap();
let root = dir.path().join("data");
let storage = Storage::new_local(root.to_str().unwrap());

storage
.put("maven/com/example/1.0/test.jar", b"artifact")
.await
.unwrap();
// Signing key under the storage root, as main.rs places it by default.
std::fs::create_dir_all(root.join(".signing")).unwrap();
std::fs::write(root.join(".signing/nora.key"), b"SECRET-SIGNING-KEY").unwrap();

let output = dir.path().join("backup.tar.gz");
let stats = create_backup(&storage, &output).await.unwrap();

// Only the real artifact is backed up, not the key.
assert_eq!(
stats.artifact_count, 1,
"signing key must not be counted or backed up as an artifact"
);

let f = File::open(&output).unwrap();
let mut archive = Archive::new(GzDecoder::new(f));
let names: Vec<String> = archive
.entries()
.unwrap()
.filter_map(|e| e.ok())
.filter_map(|e| e.path().ok().map(|p| p.to_string_lossy().into_owned()))
.collect();
assert!(
!names.iter().any(|n| n.contains(".signing")),
"backup tar must not contain the signing key: {names:?}"
);
assert!(
names.iter().any(|n| n.contains("test.jar")),
"backup tar must contain the real artifact: {names:?}"
);
}
}
52 changes: 50 additions & 2 deletions nora-registry/src/storage/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ impl LocalStorage {
if path.is_file() {
if let Ok(rel_path) = path.strip_prefix(base) {
let key = rel_path.to_string_lossy().replace('\\', "/");
if key != PIN_FILE && (key.starts_with(prefix) || prefix.is_empty()) {
if key != PIN_FILE
&& !super::is_reserved_signing_key(&key)
&& (key.starts_with(prefix) || prefix.is_empty())
{
results.push(key);
}
}
Expand Down Expand Up @@ -113,7 +116,10 @@ impl LocalStorage {
if metadata.is_file() {
if let Ok(rel_path) = path.strip_prefix(base) {
let key = rel_path.to_string_lossy().replace('\\', "/");
if key != PIN_FILE && (key.starts_with(prefix) || prefix.is_empty()) {
if key != PIN_FILE
&& !super::is_reserved_signing_key(&key)
&& (key.starts_with(prefix) || prefix.is_empty())
{
let modified = metadata
.modified()
.ok()
Expand Down Expand Up @@ -514,6 +520,48 @@ mod tests {
assert_eq!(all_keys.len(), 3);
}

#[tokio::test]
async fn list_excludes_signing_key() {
// The repository signing key lives at `<storage.path>/.signing/nora.key`
// (main.rs default) and is persisted owner-only (0600, signing.rs:182). It is
// a SECRET, not an artifact: list() must never enumerate it, or backup/migrate/
// GC/UI would leak it (0644 tarball / plaintext S3 object) or GC could delete
// the signing identity. Regression guard for the #891-class export leak.
let temp_dir = TempDir::new().unwrap();
let storage = LocalStorage::new(temp_dir.path().to_str().unwrap());

put(&storage, "npm/left-pad/-/left-pad-1.0.0.tgz", b"artifact")
.await
.unwrap();
std::fs::create_dir_all(temp_dir.path().join(".signing")).unwrap();
std::fs::write(temp_dir.path().join(".signing/nora.key"), b"SECRET-KEY").unwrap();

let all = storage.list("").await.unwrap();
assert!(
all.iter().any(|k| k.contains("left-pad")),
"real artifact must still be listed: {all:?}"
);
assert!(
!all.iter().any(|k| k.starts_with(".signing/")),
"signing key must never be enumerated by list(\"\"): {all:?}"
);

// Even an explicit prefix must not surface it.
let signing = storage.list(".signing/").await.unwrap();
assert!(
signing.is_empty(),
"explicit .signing/ prefix must still exclude the key: {signing:?}"
);

// list_with_meta shares the walk — it must exclude it too.
let meta = storage.list_with_meta("").await.unwrap();
assert!(
!meta.iter().any(|(k, _)| k.starts_with(".signing/")),
"list_with_meta must exclude the signing key: {:?}",
meta.iter().map(|(k, _)| k).collect::<Vec<_>>()
);
}

#[tokio::test]
async fn test_stat() {
let temp_dir = TempDir::new().unwrap();
Expand Down
16 changes: 16 additions & 0 deletions nora-registry/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@ use std::sync::Arc;
use thiserror::Error;
use tokio::io::AsyncRead;

/// Reserved storage prefix for the repository signing key
/// (`<storage.path>/.signing/nora.key`, see `main.rs` / [`crate::config::SigningConfig`]).
/// The key is a SECRET, persisted owner-only (0600, `signing.rs`), not an artifact:
/// every [`StorageBackend`] enumeration (`list` / `list_with_meta`) MUST skip it, so
/// backup (tar), migrate (→ object store), GC, retention and the browse UI can never
/// leak it (a 0644 tar entry / a plaintext object) or delete the signing identity.
/// Mirrors how the local pin sidecar is excluded. #891-class guard: key-creation mode
/// was enforced, the export path was not.
pub(crate) const SIGNING_KEY_PREFIX: &str = ".signing/";

/// Whether `key` names the repository signing key (anything under `.signing/`), which
/// storage enumeration must never surface. See [`SIGNING_KEY_PREFIX`].
pub(crate) fn is_reserved_signing_key(key: &str) -> bool {
key.starts_with(SIGNING_KEY_PREFIX)
}

/// File metadata
#[derive(Debug, Clone)]
pub struct FileMeta {
Expand Down
4 changes: 4 additions & 0 deletions nora-registry/src/storage/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,9 @@ impl StorageBackend for ObjectStorage {
Ok(objects
.into_iter()
.map(|meta| decode_object_key(meta.location.as_ref()))
// The signing key is a secret, never an artifact — keep it out of
// backup/migrate/GC/UI enumeration (see storage::SIGNING_KEY_PREFIX).
.filter(|key| !super::is_reserved_signing_key(key))
.collect())
}

Expand Down Expand Up @@ -372,6 +375,7 @@ impl StorageBackend for ObjectStorage {
},
)
})
.filter(|(key, _)| !super::is_reserved_signing_key(key))
.collect())
}

Expand Down
Loading