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

Be robust against unexpected files in refs/, e.g. ".latest_rsync" #1285

Closed
parasense opened this issue Oct 18, 2017 · 2 comments
Closed

Be robust against unexpected files in refs/, e.g. ".latest_rsync" #1285

parasense opened this issue Oct 18, 2017 · 2 comments

Comments

@parasense
Copy link

I observed the error when updating the summary file:

$ sudo ostree summary -u --repo=/mnt/redhat/nightly/Atomic/ostree/repo/
[sudo] password for jdisnard: 
error: No such metadata object .commit

The ostree is an rsync copy of my production repo, but contains a lot of little files named .latest_rsync , which is an artefact of how my replication tool deploys the ostree repo to a CDN. I guess some scheme to avoid re-uploading contents. Anyways, after running the above cmd under strace, I observed ostree attempting to open those files....

fcntl(12, F_GETFL)                      = 0x38800 (flags O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_NOFOLLOW)
fcntl(12, F_SETFD, FD_CLOEXEC)          = 0
getdents(12, /* 4 entries */, 32768)    = 120
openat(8, "rhel-atomic-host/7/x86_64/.latest_rsync", O_RDONLY|O_NOCTTY|O_CLOEXEC) = 13
fstat(13, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
read(13, "", 16)                        = 0
close(13)                               = 0
openat(8, "rhel-atomic-host/7/x86_64/standard", O_RDONLY|O_NOCTTY|O_CLOEXEC) = 13
fstat(13, {st_mode=S_IFREG|0644, st_size=65, ...}) = 0

so on a whim I removed all the .latest_rsync files from my copy of the repo, and then the summary file update worked afterwards. Clearly my repo was corrupted by these tiny files, and I know the fix is to simply remove them all, but I figured ostree might be made more resilient against such things.

@cgwalters cgwalters changed the title error: No such metadata object .commit Be robust against unexpected files in refs/, e.g. ".latest_rsync" Oct 18, 2017
cgwalters added a commit to cgwalters/ostree that referenced this issue Oct 18, 2017
Ignore any files that start with a `.` under the assumption they're metadata for
some other tool. This is a bit ugly as `.` *is* valid in refs, but I don't see
many people using that in the wild.

Closes: ostreedev#1285
cgwalters added a commit to cgwalters/ostree that referenced this issue Oct 18, 2017
Ignore any files that start with a `.` under the assumption they're metadata for
some other tool. This is a bit ugly as `.` *is* valid in refs, but I don't see
many people using that in the wild. And we're just ignoring ones starting with
`.` which would be even more unusual, as that's the Unix hidden file convention.

Closes: ostreedev#1285
@cgwalters
Copy link
Member

Sure, easy enough. PR in #1286

@parasense
Copy link
Author

Thanks, and meanwhile I'm filing a bug against my deployment tool, to ask they stop using dot files and perhaps use extended attributes or some other scheme to track uploads. Appreciate the quick turn around, thanks again.

cgwalters added a commit to cgwalters/ostree that referenced this issue Oct 18, 2017
Change the regexp for validating refs to require at least one letter or digit
before allowing the other special chars in the set `[.-_]`. Names that start
with `.` are traditionally Unix hidden files; let's ignore them under the
assumption they're metadata for some other tool, and we don't want to
potentially conflict with the special `.` and `..` Unix directory entries.
Further, names starting with `-` are problematic for Unix cmdline option
processing; there's no good reason to support that. Finally, disallow `_` just
on general principle - it's simpler to say that ref identifiers must start with
a letter or digit.

We also ignore any existing files (that might be previously created refs) that
start with `.` in the `refs/` directory - there's a Red Hat tool for content
management that injects `.rsync` files, which is why this patch was first
written.

V1: Update to ban all refs starting with a non-letter/digit, and
    also add another call to `ostree_validate_rev` in the pull
    code.

Closes: ostreedev#1285
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

2 participants