binpash / binpash/try

Handle `$mountpoint == /` better

Open
#220 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Shell
Stars
5.5k
Forks
81
Avg merge
1m
Merged PRs (30d)
1

Description

There exists an edge case when creating the contents of hidedir:

https://github.com/binpash/try/blob/619b0a20cf0e3f38d78f9a3b2d1680c848d2f860/try#L156

The `$(stat -c %a "$mountpoint")` command gets the permissions of `$mountpoint` as an octal triplet (e.g. 755) which is then passed as the permissions to create the folder for that directory under `hidedir`. I encountered this edge case when running the test suite, in particular `hide_flag.sh`, because it attempts to operate with a mountpoint set to `/`, which on my system has permissions of 555. The problem here is twofold:
- This line attempts to create `.../hidedir//` which simplifies `.../hidedir` itself
- It therefore sets the permissions on *hidedir*, not on the *mountpoint* underneath it, to 555, which doesn't allow writing anything into it.

A quick fix is to simply append `$SANDBOX_DIR/hidedir` to the list of pre-created directories earlier in the workflow:

https://github.com/binpash/try/blob/619b0a20cf0e3f38d78f9a3b2d1680c848d2f860/try#L104

This creates hidedir with write permissions, so later operations succeed and the test passes. But I'm not sure whether this is the right approach, because it's not clear to me that having `.../hidedir/bin`, `.../hidedir/etc`, and so on is correct. It looks like the code expects the contents of `hidedir` to be whatever was in `$DIRS_AND_MOUNTS` (here `/`) and instead we're putting `/`'s child files and folders directly into it without the expected intermediate level. So applying the quick fix I found lets the test suite pass, but if that's just masking the problem of not handling `/` properly, then it will take a more involved approach than I understand how to write right now.

For reference here is the log of the failing test.

```
Running hide_flag.sh... FAIL (3)
=======
STDOUT:
=======
=======

=======
STDERR:
=======
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//boot’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//dev’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//etc’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//home’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//lost+found’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//mnt’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//opt’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//proc’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//root’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//run’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//srv’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//sys’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//tmp’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//usr’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//var’: Permission denied
mkdir: cannot create directory ‘/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//tmp’: Permission denied
touch: cannot touch '/tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//tmp/tmp.mXz83rZUkL/hidden_file': No such file or directory
setfattr: /tmp/tmp.nX5htJY95A.try-1782959338286/hidedir//tmp/tmp.mXz83rZUkL/hidden_file: No such file or directory
try: could not create whiteout for hidden path '/tmp/tmp.mXz83rZUkL/hidden_file'
=======
```

and here are the permissions:

```
%stat -c '%a %n' /tmp/tmp.nX5htJY95A.try-1782959338286/*
600 /tmp/tmp.nX5htJY95A.try-1782959338286/exclude
600 /tmp/tmp.nX5htJY95A.try-1782959338286/hide
555 /tmp/tmp.nX5htJY95A.try-1782959338286/hidedir
600 /tmp/tmp.nX5htJY95A.try-1782959338286/include
644 /tmp/tmp.nX5htJY95A.try-1782959338286/mounts
644 /tmp/tmp.nX5htJY95A.try-1782959338286/mounts.updated
755 /tmp/tmp.nX5htJY95A.try-1782959338286/temproot
755 /tmp/tmp.nX5htJY95A.try-1782959338286/upperdir
755 /tmp/tmp.nX5htJY95A.try-1782959338286/workdir
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Inspect the `try` script around lines 104 and 156, then reproduce the failure with the `hide_flag.sh` test when the mountpoint is `/`. Trace how the root path is mapped beneath `hidedir` and determine the intended intermediate directory layout. Done means the test passes without making `hidedir` unwritable and the generated paths preserve the expected mapping.

Written by the indexing model from the issue text.

Assessment

Tech stack
shell
Domain
cli, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.