Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignition should fail validation if a file conflicts with the parent directory of another file/link/dir #1457

Open
marmijo opened this issue Sep 1, 2022 · 8 comments
Assignees

Comments

@marmijo
Copy link
Contributor

marmijo commented Sep 1, 2022

Bug

Operating System Version

Any

Ignition Version

All

Environment

All

Expected Behavior

Ignition should not allow internally inconsistent configs

Actual Behavior

Users can specify a file that would conflict with the parent directory of another file/link/dir. Ignition should fail validation if files/links/dirs conflict with implicit parent directories.

Reproduction Steps

1 - Write a config with a file at /foo/bar and a file /foo/bar/baz
OR
1 - Write a config with a file at /foo/bar and a directory at /foo/bar/baz
OR
1 - Write a config with a file at /foo/bar and a link at /foo/bar/baz

Other Information

On top of general validation for this issue, the validation added by Ignition PR#1456 may need to be updated to include the fix as well.

@yasminvalim
Copy link
Contributor

Hey @marmijo, could you please provide more context about this?
I'm trying to understand the reproduction steps more clearly.
Do I need to write a configuration using Butane and then check the resulting Ignition file?

Thank you! 😸

@marmijo
Copy link
Contributor Author

marmijo commented Jan 18, 2024

Hi Yasmin. Yes, you could use butane to create the ignition config. To reproduce, you'll need the resulting ignition config that has the file/link/dir conflicts specified like the ones above and run it through ignition validation. It currently doesn't fail validation but it should. It should be noted that butane isn't needed to write an ignition config, so ignition validation code is needed here.

Basically, it's possible to create an ignition config with two files that conflict with one another in such a way that one file would be an implicit parent directory of the other e.g. /var/home/core/log (as a file) and /var/home/core/log/output.txt (which now declares log as a directory).

@prestist
Copy link
Collaborator

Note: when we add this verification, we need to try and not reduce functionality.

Its completely reasonable to have a file /home/bar/zed and a dir home/bar/zed/
with a file in it at /home/bar/zed/zed

@marmijo
Copy link
Contributor Author

marmijo commented Jan 30, 2024

AFAIK, Linux disallows dirs/files/links/pipes with the same name in the same directory. It shouldn't be possible to create /home/bar/zed/zed if /home/bar/zed already exists as a file/link.
The same is true for creating a file/link at /home/bar/zed when /home/bar/zed/zed already exists.

@prestist
Copy link
Collaborator

@marmijo Wdyt about simply validating that files end with an extension ?
I think it solves thie issue more directly. and more cleanly. Checking for collisions feels clumsy.

@jlebon
Copy link
Member

jlebon commented Feb 28, 2024

@prestist That doesn't really fix it. It's perfectly valid for directories to have "extensions" and files not to.

I don't think it'd be too hard to validate this. See e.g. getOrderedCreationList() which already knows how to sort paths from most shallow to deepest. So you can sort all entries, then go down the list from the top. For each entry, check if its path is within the path of the previous entry. If so, verify that the previous entry is a directory. If not, fail.

Obviously, this validation can't catch symlinks that only exist on the host (e.g. if /foo/bar is a symlink to /foo/baz and the user wants a file at /foo/baz and a file at /foo/bar/baz). Those will just fail at runtime like today.

@prestist
Copy link
Collaborator

prestist commented Mar 4, 2024

@jlebon ah, okay that was a wiff on my part, I assumed that directorys would not have extensions, what purpose does that serve for (my own knowledge) ?

Thats great, thank you for the pointer to getOrderedCreationList. @yasminvalim and my self were going through some of her tests, and I started stumbling a bit on the tests, because I was thinking that it could be possible that the directory is externally derived, however if we are using the config as truth, then with the example 1 - Write a config with a file at /foo/bar and a directory at /foo/bar/baz if we instead had
1 - Write a config with a file at /foo/bar and a directory at /foo/bar/baz and a directory at /foo/bar would be valid right?
otherwise we assume user error?

@jlebon
Copy link
Member

jlebon commented Mar 13, 2024

@jlebon ah, okay that was a wiff on my part, I assumed that directorys would not have extensions, what purpose does that serve for (my own knowledge) ?

The base name is mostly an arbitrary string. Indeed, by convention files tend to have extensions and directories tend not to. An common example of directories with an extension are the systemd dropin dirs (i.e. <service>.d or <target>.wants). You can run something like find / -name '*.*' -type d on your system if you want to find more examples.

if we are using the config as truth, then with the example 1 - Write a config with a file at /foo/bar and a directory at /foo/bar/baz if we instead had 1 - Write a config with a file at /foo/bar and a directory at /foo/bar/baz and a directory at /foo/bar would be valid right?

That would give duplicate entries which should already fail validation today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants