fix(cli): reject misplaced/unknown manifest keys (§5.5 review)
`extension_points` is an `[app]`/`[group]` node key. Placed at top level (before any table header) TOML reads it as a root key — and `Manifest` silently dropped unknown root keys, the same silent-drop class that bit in C5 (a key absorbed under `[group]` and discarded). Add `deny_unknown_fields` to `Manifest`, `ManifestApp`, and `ManifestGroup` so a misplaced or typo'd key is a hard parse error instead of a no-op apply. Regression test covers top-level placement, an in-`[app]` typo, and the correct node-key form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -29,6 +29,7 @@ use serde::{Deserialize, Serialize};
|
|||||||
pub const MANIFEST_FILE: &str = "picloud.toml";
|
pub const MANIFEST_FILE: &str = "picloud.toml";
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||||
|
#[serde(deny_unknown_fields)]
|
||||||
pub struct Manifest {
|
pub struct Manifest {
|
||||||
/// An app node declares `[app]`; a group node declares `[group]` (Phase 5).
|
/// An app node declares `[app]`; a group node declares `[group]` (Phase 5).
|
||||||
/// Exactly one is present (enforced by [`Manifest::parse`]).
|
/// Exactly one is present (enforced by [`Manifest::parse`]).
|
||||||
@@ -209,6 +210,7 @@ pub struct OverlayApp {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||||
|
#[serde(deny_unknown_fields)]
|
||||||
pub struct ManifestApp {
|
pub struct ManifestApp {
|
||||||
pub slug: String,
|
pub slug: String,
|
||||||
pub name: String,
|
pub name: String,
|
||||||
@@ -227,6 +229,7 @@ pub struct ManifestApp {
|
|||||||
/// `pic groups create`); the manifest reconciles its content, not the tree
|
/// `pic groups create`); the manifest reconciles its content, not the tree
|
||||||
/// shape. In a nested project the parent is inferred from the directory tree.
|
/// shape. In a nested project the parent is inferred from the directory tree.
|
||||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||||
|
#[serde(deny_unknown_fields)]
|
||||||
pub struct ManifestGroup {
|
pub struct ManifestGroup {
|
||||||
pub slug: String,
|
pub slug: String,
|
||||||
pub name: String,
|
pub name: String,
|
||||||
@@ -624,6 +627,29 @@ mod tests {
|
|||||||
assert_eq!(m, Manifest::parse(&text).unwrap());
|
assert_eq!(m, Manifest::parse(&text).unwrap());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_misplaced_or_unknown_keys() {
|
||||||
|
// §5.5 footgun guard: `extension_points` is an `[app]`/`[group]` node
|
||||||
|
// key. Placed at top level (before any table header) TOML reads it as a
|
||||||
|
// root key — `deny_unknown_fields` must reject it loudly rather than
|
||||||
|
// silently drop it (the silent-drop bug that bit in C5).
|
||||||
|
let misplaced = "extension_points = [\"theme\"]\n[app]\nslug = \"x\"\nname = \"X\"\n";
|
||||||
|
assert!(
|
||||||
|
Manifest::parse(misplaced).is_err(),
|
||||||
|
"a top-level extension_points must be rejected, not silently ignored"
|
||||||
|
);
|
||||||
|
// A typo'd key inside [app] is likewise a hard error, not a drop.
|
||||||
|
let typo = "[app]\nslug = \"x\"\nname = \"X\"\nextention_points = [\"theme\"]\n";
|
||||||
|
assert!(
|
||||||
|
Manifest::parse(typo).is_err(),
|
||||||
|
"an unknown key in [app] must be rejected"
|
||||||
|
);
|
||||||
|
// The correct node-key placement still parses.
|
||||||
|
let ok = "[app]\nslug = \"x\"\nname = \"X\"\nextension_points = [\"theme\"]\n";
|
||||||
|
let m = Manifest::parse(ok).expect("node-key extension_points must parse");
|
||||||
|
assert_eq!(m.extension_points(), ["theme"]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn group_manifest_parses_and_rejects_app_only_blocks() {
|
fn group_manifest_parses_and_rejects_app_only_blocks() {
|
||||||
// A [group] node: scripts + vars, no [app].
|
// A [group] node: scripts + vars, no [app].
|
||||||
|
|||||||
Reference in New Issue
Block a user