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
36 changes: 36 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,39 @@ jobs:
--no-write-lock-file '.#contents.includingOutputPaths' \
| nix run nixpkgs\#gron > corpus/DeterminateSystems/fh/=0.1.12/including.output
diff corpus/DeterminateSystems/fh/=0.1.12/including.expect corpus/DeterminateSystems/fh/=0.1.12/including.output

- name: Ensure evaluating tests/bySystem/0.1.4 without output paths produces the right result
if: success() || failure()
run: |
nix eval --json \
--override-input flake ./tests/bySystem/0.1.4 \
--no-write-lock-file '.#contents.excludingOutputPaths' \
| nix run nixpkgs\#gron > corpus/tests/bySystem/0.1.4/excluding.output
diff corpus/tests/bySystem/0.1.4/excluding.expect corpus/tests/bySystem/0.1.4/excluding.output

- name: Ensure evaluating tests/bySystem/0.1.4 with output paths produces the right result
if: success() || failure()
run: |
nix eval --json \
--override-input flake ./tests/bySystem/0.1.4 \
--no-write-lock-file '.#contents.includingOutputPaths' \
| nix run nixpkgs\#gron > corpus/tests/bySystem/0.1.4/including.output
diff corpus/tests/bySystem/0.1.4/including.expect corpus/tests/bySystem/0.1.4/including.output

- name: Ensure evaluating tests/bySystem/0.3.0 without output paths produces the right result
if: success() || failure()
run: |
nix eval --json \
--override-input flake ./tests/bySystem/0.3.0 \
--no-write-lock-file '.#contents.excludingOutputPaths' \
| nix run nixpkgs\#gron > corpus/tests/bySystem/0.3.0/excluding.output
diff corpus/tests/bySystem/0.3.0/excluding.expect corpus/tests/bySystem/0.3.0/excluding.output

- name: Ensure evaluating tests/bySystem/0.3.0 with output paths produces the right result
if: success() || failure()
run: |
nix eval --json \
--override-input flake ./tests/bySystem/0.3.0 \
--no-write-lock-file '.#contents.includingOutputPaths' \
| nix run nixpkgs\#gron > corpus/tests/bySystem/0.3.0/including.output
diff corpus/tests/bySystem/0.3.0/including.expect corpus/tests/bySystem/0.3.0/including.output
17 changes: 17 additions & 0 deletions corpus/tests/bySystem/0.1.4/excluding.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
json = {};
json.docs = {};
json.docs.bySystem = "The `bySystem` flake output defines a per-system output, like a flake's `formatter`.\n";
json.inventory = {};
json.inventory.bySystem = {};
json.inventory.bySystem.children = {};
json.inventory.bySystem.children["aarch64-darwin"] = {};
json.inventory.bySystem.children["aarch64-darwin"].forSystems = [];
json.inventory.bySystem.children["aarch64-darwin"].forSystems[0] = "aarch64-darwin";
json.inventory.bySystem.children["aarch64-darwin"].shortDescription = "";
json.inventory.bySystem.children["aarch64-darwin"].what = "bySystem";
json.inventory.bySystem.children["x86_64-linux"] = {};
json.inventory.bySystem.children["x86_64-linux"].forSystems = [];
json.inventory.bySystem.children["x86_64-linux"].forSystems[0] = "x86_64-linux";
json.inventory.bySystem.children["x86_64-linux"].shortDescription = "";
json.inventory.bySystem.children["x86_64-linux"].what = "bySystem";
json.version = 1;
25 changes: 25 additions & 0 deletions corpus/tests/bySystem/0.1.4/including.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
json = {};
json.docs = {};
json.docs.bySystem = "The `bySystem` flake output defines a per-system output, like a flake's `formatter`.\n";
json.inventory = {};
json.inventory.bySystem = {};
json.inventory.bySystem.children = {};
json.inventory.bySystem.children["aarch64-darwin"] = {};
json.inventory.bySystem.children["aarch64-darwin"].derivation = "/nix/store/0hbva7czr00sf4m97jlw4swagc8ccz87-simple.drv";
json.inventory.bySystem.children["aarch64-darwin"].forSystems = [];
json.inventory.bySystem.children["aarch64-darwin"].forSystems[0] = "aarch64-darwin";
json.inventory.bySystem.children["aarch64-darwin"].outputs = {};
json.inventory.bySystem.children["aarch64-darwin"].outputs.doc = "/nix/store/3jvc25bixqghypgda09hhg7sm2sdvi1k-simple-doc";
json.inventory.bySystem.children["aarch64-darwin"].outputs.out = "/nix/store/9nnh702lr0klxsgsqff3p198ycxa2gbj-simple";
json.inventory.bySystem.children["aarch64-darwin"].shortDescription = "";
json.inventory.bySystem.children["aarch64-darwin"].what = "bySystem";
json.inventory.bySystem.children["x86_64-linux"] = {};
json.inventory.bySystem.children["x86_64-linux"].derivation = "/nix/store/pxlb58dhphzbzhk40gbasdlaxnrjxndr-simple.drv";
json.inventory.bySystem.children["x86_64-linux"].forSystems = [];
json.inventory.bySystem.children["x86_64-linux"].forSystems[0] = "x86_64-linux";
json.inventory.bySystem.children["x86_64-linux"].outputs = {};
json.inventory.bySystem.children["x86_64-linux"].outputs.doc = "/nix/store/5yf9i39g2kk2xs7kzwq3v3pcar92r47z-simple-doc";
json.inventory.bySystem.children["x86_64-linux"].outputs.out = "/nix/store/bc36n9f0as5lxkq35xyvc2ch40j7y3ya-simple";
json.inventory.bySystem.children["x86_64-linux"].shortDescription = "";
json.inventory.bySystem.children["x86_64-linux"].what = "bySystem";
json.version = 1;
17 changes: 17 additions & 0 deletions corpus/tests/bySystem/0.3.0/excluding.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
json = {};
json.docs = {};
json.docs.bySystem = "The `bySystem` flake output defines a per-system output, like a flake's `formatter`.\n";
json.inventory = {};
json.inventory.bySystem = {};
json.inventory.bySystem.children = {};
json.inventory.bySystem.children["aarch64-darwin"] = {};
json.inventory.bySystem.children["aarch64-darwin"].forSystems = [];
json.inventory.bySystem.children["aarch64-darwin"].forSystems[0] = "aarch64-darwin";
json.inventory.bySystem.children["aarch64-darwin"].shortDescription = "";
json.inventory.bySystem.children["aarch64-darwin"].what = "bySystem";
json.inventory.bySystem.children["x86_64-linux"] = {};
json.inventory.bySystem.children["x86_64-linux"].forSystems = [];
json.inventory.bySystem.children["x86_64-linux"].forSystems[0] = "x86_64-linux";
json.inventory.bySystem.children["x86_64-linux"].shortDescription = "";
json.inventory.bySystem.children["x86_64-linux"].what = "bySystem";
json.version = 1;
25 changes: 25 additions & 0 deletions corpus/tests/bySystem/0.3.0/including.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
json = {};
json.docs = {};
json.docs.bySystem = "The `bySystem` flake output defines a per-system output, like a flake's `formatter`.\n";
json.inventory = {};
json.inventory.bySystem = {};
json.inventory.bySystem.children = {};
json.inventory.bySystem.children["aarch64-darwin"] = {};
json.inventory.bySystem.children["aarch64-darwin"].derivation = "/nix/store/0hbva7czr00sf4m97jlw4swagc8ccz87-simple.drv";
json.inventory.bySystem.children["aarch64-darwin"].forSystems = [];
json.inventory.bySystem.children["aarch64-darwin"].forSystems[0] = "aarch64-darwin";
json.inventory.bySystem.children["aarch64-darwin"].outputs = {};
json.inventory.bySystem.children["aarch64-darwin"].outputs.doc = "/nix/store/3jvc25bixqghypgda09hhg7sm2sdvi1k-simple-doc";
json.inventory.bySystem.children["aarch64-darwin"].outputs.out = "/nix/store/9nnh702lr0klxsgsqff3p198ycxa2gbj-simple";
json.inventory.bySystem.children["aarch64-darwin"].shortDescription = "";
json.inventory.bySystem.children["aarch64-darwin"].what = "bySystem";
json.inventory.bySystem.children["x86_64-linux"] = {};
json.inventory.bySystem.children["x86_64-linux"].derivation = "/nix/store/pxlb58dhphzbzhk40gbasdlaxnrjxndr-simple.drv";
json.inventory.bySystem.children["x86_64-linux"].forSystems = [];
json.inventory.bySystem.children["x86_64-linux"].forSystems[0] = "x86_64-linux";
json.inventory.bySystem.children["x86_64-linux"].outputs = {};
json.inventory.bySystem.children["x86_64-linux"].outputs.doc = "/nix/store/5yf9i39g2kk2xs7kzwq3v3pcar92r47z-simple-doc";
json.inventory.bySystem.children["x86_64-linux"].outputs.out = "/nix/store/bc36n9f0as5lxkq35xyvc2ch40j7y3ya-simple";
json.inventory.bySystem.children["x86_64-linux"].shortDescription = "";
json.inventory.bySystem.children["x86_64-linux"].what = "bySystem";
json.version = 1;
84 changes: 55 additions & 29 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
inputs.flake.url = "https://flakehub.com/f/DeterminateSystems/flake-schemas/*";

outputs = inputs:
let
getFlakeOutputs = flake: includeOutputPaths:
Expand All @@ -9,13 +10,24 @@

mapAttrsToList = f: attrs: map (name: f name attrs.${name}) (builtins.attrNames attrs);

getAttrFromPath = paths: obj:
let
length = builtins.length paths - 1;

go = acc: index:
if index > length
then acc
else go acc.${builtins.elemAt paths index} (index + 1);
in
go obj 0;

try = e: default:
let res = builtins.tryEval e;
in if res.success then res.value else default;

mkChildren = children: { inherit children; };

flakeSchemasFlakePinned = "https://api.flakehub.com/f/pinned/DeterminateSystems/flake-schemas/0.1.4/0190e653-dd76-70bd-ba6e-a3f5eaf3d415/source.tar.gz?narHash=sha256-efoDF3VaZHpcwFd2Y1axGLqNX/ou9kDL7z9mWNqzv9w%3D";
flakeSchemasFlakePinned = "https://api.flakehub.com/f/pinned/DeterminateSystems/flake-schemas/0.3.0/019c9f61-e746-760e-a1fe-53f05b10d026/source.tar.gz?narHash=sha256-hcUPpu25%2BVLvQsf961cu4zTeA//Ab35MaMjqSS/Ojqc%3D";
in

rec {
Expand Down Expand Up @@ -52,53 +64,67 @@
uncheckedOutputs =
builtins.filter (outputName: ! schemas ? ${outputName}) (builtins.attrNames flake.outputs);

inventoryFor = filterFun:
inventoryFor =
filterFun:
builtins.mapAttrs
(outputName: schema:
(
outputName: schema:
let
doFilter = attrs:
if filterFun attrs
then
if attrs ? children
then
mkChildren (builtins.mapAttrs (childName: child: doFilter child) attrs.children)
doFilter =
attrs: output:
if filterFun attrs then
if attrs ? children then
mkChildren
(
builtins.mapAttrs (childName: child: doFilter child output.${childName}) attrs.children
)
Comment on lines +73 to +80
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only real change here is passing attrs.children to doFilter inside mapAttrs.

else
{
forSystems = attrs.forSystems or null;
shortDescription = attrs.shortDescription or null;
what = attrs.what or null;
#evalChecks = attrs.evalChecks or {};
} // (
if includeOutputPaths then
{
derivation =
if attrs ? derivation
then builtins.unsafeDiscardStringContext attrs.derivation.drvPath
}
// (
if
includeOutputPaths then
let
drv =
if attrs?derivationAttrPath
then getAttrFromPath attrs.derivationAttrPath output
# TODO: This is a fallback for legacy behavior.
# Remove when most (or all) flake schemas in the
# wild use derivationAttrPath instead of
# derivation.
else if attrs?derivation
then attrs.derivation
else null;
in
{
derivation = if drv != null then builtins.unsafeDiscardStringContext drv.drvPath else null;
outputs =
if attrs ? derivation
then
if drv != null then
builtins.listToAttrs
(
builtins.map
(outputName:
{
name = outputName;
value = attrs.derivation.${outputName}.outPath;
}
)
attrs.derivation.outputs
)
else
null;
(outputName: {
name = outputName;
value = drv.${outputName}.outPath;
})
drv.outputs
) else null;
Comment on lines +91 to +114
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard derivation path dereferences to prevent full inventory evaluation failure.

Line 93, Line 100, and Line 108 can throw when a path is stale or resolves to a non-derivation value. That makes includingOutputPaths fail hard instead of returning nullable metadata for the problematic node.

🛡️ Suggested hardening patch
-                              drv =
-                                if attrs?derivationAttrPath
-                                then getAttrFromPath attrs.derivationAttrPath output
+                              drv =
+                                if attrs ? derivationAttrPath
+                                then try (getAttrFromPath attrs.derivationAttrPath output) null
                                 # TODO: remove when no longer needed
-                                else if attrs?derivation
+                                else if attrs ? derivation
                                 then attrs.derivation
                                 else null;
                             in
                             {
-                              derivation = if drv != null then drv.drvPath else null;
+                              derivation = try (if drv != null then drv.drvPath else null) null;
                               outputs =
-                                if drv != null then
-                                  builtins.listToAttrs
-                                    (
-                                      builtins.map
-                                        (outputName: {
-                                          name = outputName;
-                                          value = drv.${outputName}.outPath;
-                                        })
-                                        drv.outputs
-                                    ) else null;
+                                try (
+                                  if drv != null then
+                                    builtins.listToAttrs
+                                      (
+                                        builtins.map
+                                          (outputName: {
+                                            name = outputName;
+                                            value = drv.${outputName}.outPath;
+                                          })
+                                          drv.outputs
+                                      )
+                                  else null
+                                ) null;
                             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
drv =
if attrs?derivationAttrPath
then getAttrFromPath attrs.derivationAttrPath output
# TODO: remove when no longer needed
else if attrs?derivation
then attrs.derivation
else null;
in
{
derivation = if drv != null then drv.drvPath else null;
outputs =
if attrs ? derivation
then
if drv != null then
builtins.listToAttrs
(
builtins.map
(outputName:
{
name = outputName;
value = attrs.derivation.${outputName}.outPath;
}
)
attrs.derivation.outputs
)
else
null;
(outputName: {
name = outputName;
value = drv.${outputName}.outPath;
})
drv.outputs
) else null;
drv =
if attrs ? derivationAttrPath
then try (getAttrFromPath attrs.derivationAttrPath output) null
# TODO: remove when no longer needed
else if attrs ? derivation
then attrs.derivation
else null;
in
{
derivation = try (if drv != null then drv.drvPath else null) null;
outputs =
try (
if drv != null then
builtins.listToAttrs
(
builtins.map
(outputName: {
name = outputName;
value = drv.${outputName}.outPath;
})
drv.outputs
)
else null
) null;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@flake.nix` around lines 91 - 111, The code dereferences
attrs.derivationAttrPath, attrs.derivation and fields on drv (drv.drvPath,
drv.outputs, drv.${outputName}.outPath) without guarding, which can throw and
make includingOutputPaths fail; fix by validating before access (e.g. use
builtins.hasAttr / builtins.tryEval or builtins.isDerivation where available) so
drv is only treated as a derivation when it actually has drvPath and outputs,
and when mapping outputs check each outputName exists on drv before reading
drv.${outputName}.outPath; update the branches that compute drv and the
derivation/outputs results to return null/nullable metadata when validation
fails instead of performing direct dereferences.

}
else
{ }
)
else
{ };
in
doFilter ((schema.inventory or (output: { })) flake.outputs.${outputName})
doFilter
((schema.inventory or (output: { }))
flake.outputs.${outputName}
)
flake.outputs.${outputName}

)
schemas;

Expand Down
65 changes: 65 additions & 0 deletions tests/bySystem/0.1.4/flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{
description = "Flake with a custom schema mimicking `formatter`, using flake-schema 0.1.4's behavior";

inputs = { };

outputs =
_:
let
nameValuePair = name: value: { inherit name value; };
genAttrs = names: f: builtins.listToAttrs (map (n: nameValuePair n (f n)) names);

stubFor =
system:
name:
derivation {
inherit name system;

builder = "/bin/sh";
args = [
"-c"
''
echo "out: $name/$system" >$out
echo "doc: $name/$system" >$doc
''
];

outputs = [
"out"
"doc"
];
};

systems = [
"aarch64-darwin"
"x86_64-linux"
];

forEachSystem = f: genAttrs systems (system: f (stubFor system));
in
{
# A flat schema like formatter.${system}
schemas.bySystem = {
version = 1;
doc = ''
The `bySystem` flake output defines a per-system output, like a flake's `formatter`.
'';

appendSystem = true;
defaultAttrPath = [ ];

inventory = output: {
children = builtins.mapAttrs
(system: pkg: {
what = "bySystem";
forSystems = [ system ];
shortDescription = pkg.meta.description or "";
derivation = pkg;
})
output;
};
};

bySystem = forEachSystem (stub: stub "simple");
};
}
65 changes: 65 additions & 0 deletions tests/bySystem/0.3.0/flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{
description = "Flake with a custom schema mimicking `formatter`, using flake-schema 0.3.0's behavior";

inputs = { };

outputs =
_:
let
nameValuePair = name: value: { inherit name value; };
genAttrs = names: f: builtins.listToAttrs (map (n: nameValuePair n (f n)) names);

stubFor =
system:
name:
derivation {
inherit name system;

builder = "/bin/sh";
args = [
"-c"
''
echo "out: $name/$system" >$out
echo "doc: $name/$system" >$doc
''
];

outputs = [
"out"
"doc"
];
};

systems = [
"aarch64-darwin"
"x86_64-linux"
];

forEachSystem = f: genAttrs systems (system: f (stubFor system));
in
{
# A flat schema like formatter.${system}
schemas.bySystem = {
version = 1;
doc = ''
The `bySystem` flake output defines a per-system output, like a flake's `formatter`.
'';

appendSystem = true;
defaultAttrPath = [ ];

inventory = output: {
children = builtins.mapAttrs
(system: pkg: {
what = "bySystem";
forSystems = [ system ];
shortDescription = pkg.meta.description or "";
derivationAttrPath = [ ];
})
output;
};
};

bySystem = forEachSystem (stub: stub "simple");
};
}