Skip to content

mount: check for more requisite mountpoint conditions#5718

Merged
MichaelEischer merged 4 commits into
restic:masterfrom
jtru:mount-fail-early-with-unsuitable-mp
Feb 19, 2026
Merged

mount: check for more requisite mountpoint conditions#5718
MichaelEischer merged 4 commits into
restic:masterfrom
jtru:mount-fail-early-with-unsuitable-mp

Conversation

@jtru

@jtru jtru commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

What does this PR change? What problem does it solve?

Esp. with large repositories, restic mount may waste a lot of time and resources on preparing for a fuse mount that ultimately cannot succeed due conditions that are not checked for.

In order to be able to mount a repository over a mountpoint target directory via FUSE, that target directory needs to be both writeable and executable for the UID performing the mount, which this patch ensures.

FUSE does allow for mounting over a target path that refers to a regular (writeable) file, but the result is not accessible via chdir(), so we prevent that as well, and accept only directory inodes as the intended target mountpoint path.

Was the change previously discussed in an issue or on the forum?

No.

Checklist

  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I'm done! This pull request is ready for review.

In order to be able to mount a repository over a mountpoint target
directory via FUSE, that target directory needs to be both writeable and
executable for the UID performing the mount.

Without this patch, `restic mount` only checks for the target pathname's
existence, which can lead to a lot of data transfer and/or computation
for large repos to be performed before eventually croaking with a fatal
"fusermount: failed to chdir to mountpoint: Permission denied" (or
similar) error.

FUSE does allow for mounting over a target path that refers to a regular
(writeable) file, but the result is not accessible via chdir(), so we
prevent that as well, and accept only directory inodes as the intended
target mountpoint path.

@MichaelEischer MichaelEischer left a comment

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.

Please add a small changelog entry, see https://github.com/restic/restic/blob/master/changelog/TEMPLATE for the template.

Comment thread cmd/restic/cmd_mount.go Outdated
// Check the existence of the mount point at the earliest stage to
// prevent unnecessary computations while opening the repository.
if _, err := os.Stat(mountpoint); errors.Is(err, os.ErrNotExist) {
mountpoint_stat, err := os.Stat(mountpoint)

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.

Suggested change
mountpoint_stat, err := os.Stat(mountpoint)
stat, err := os.Stat(mountpoint)

Or mountpointStat. Go uses camel case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ack, I pushed a change to rectify that :)

I hope the changelog entry is satisfactory!

@jtru jtru requested a review from MichaelEischer February 19, 2026 07:27

@MichaelEischer MichaelEischer left a comment

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.

LGTM. Thanks! I've slightly tweaked the changelog summary, we reserve change for breaking changes.

@MichaelEischer MichaelEischer enabled auto-merge (squash) February 19, 2026 17:04
@MichaelEischer MichaelEischer merged commit a8f0ad5 into restic:master Feb 19, 2026
11 checks passed
@jtru jtru deleted the mount-fail-early-with-unsuitable-mp branch February 19, 2026 17:26
@jtru

jtru commented Feb 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks very much for merging!

Apologies for causing the small bit of extra work - I wasn't aware of the special meaning of these words in the changelog context and mistook "change" for the most neutral/innocent of the three options ;)

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

Successfully merging this pull request may close these issues.

2 participants