Skip to content

prevent tunnel sockets from overwriting existing files - #1141

Open
shizuku198411 wants to merge 2 commits into
canonical:mainfrom
shizuku198411:fix/tunnel-prevent-file-overwrite
Open

shizuku198411 wants to merge 2 commits into
canonical:mainfrom
shizuku198411:fix/tunnel-prevent-file-overwrite

Conversation

@shizuku198411

Copy link
Copy Markdown
Contributor

Description

Fixes #1140

Prevent the tunnel interface from overwriting existing files when creating a Unix socket.

Previously, Workshop could replace an existing regular file with a Unix socket during tunnel setup, potentially causing data loss.

This PR adds an os.Lstat() check to reject socket creation if the target path already exists, regardless of its file type.

Changes:

  • Reject existing paths, including regular files, directories, symlinks, and Unix sockets.
  • Add unit tests covering each case and nonexistent paths.
  • Return a clear error without modifying existing files.

Verification:

  • Confirmed that existing regular files are no longer overwritten.
  • Confirmed that Unix sockets are created successfully when the target path does not exist.
  • Verified that start, stop, and refresh continue to work as expected.

Self-review quick check

  • Make decisions that cost a lot to reverse explicit in the PR description.
  • Avoid nested conditions.
  • Delete dead code and redundant comments.
  • Normalise symmetries by sticking to doing identical things identically.
  • Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
  • Put variable declaration and initialisation together.
  • Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
  • Put a blank line between two logically different chunks of code.
  • Follow the style guide for new error messages.

Docs

  • I confirm the PR has no implications for documentation.

@jonathan-conder

Copy link
Copy Markdown
Contributor

I think we should overwrite the file if it's another socket; it could be left over from another session that closed abruptly. But if it's a regular file we should probably avoid deleting it, yes.

@shizuku198411

Copy link
Copy Markdown
Contributor Author

I agree, allowing existing sockets to be overwritten makes sense.
I've updated the commit accordingly, while ensuring that regular files are protected from being overwritten.

This branch has not been deployed

No deployments
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.

Tunnel interface overwrites existing files when creating Unix sockets

2 participants