Feature/filesystem mapping by KesterTan · Pull Request #2309 · autolab/Autolab

KesterTan · 2026-02-02T21:54:59Z

This PR provides a way to map the current files and folders in Autolab to a specified user group associated with the instructor's email. This PR also allows users to provide SSH keys so that they can SSH to the server hosting Autolab.

Instructors should not be able to see courses that they do not manage, but they should be able to access and view courses that they do manage.

How Has This Been Tested?

Login to the gamma instance and create a course. Upload your ssh key by clicking your profile, then your account and then manage ssh keys. SSH to the instance and check that you can only access your course and not other courses.

Types of changes

[] Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have run rubocop and erblint for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
My change requires a change to the documentation, which is located at Autolab Docs
I have updated the documentation accordingly, included in this PR

… web api function to enforce it

- Add SshKey model with validation and fingerprinting - Implement UnixGroupManager service for user/group management - Create Unix users/groups automatically on course/staff creation - Add SSH key provisioning to authorized_keys - Implement FilesystemEnforcer with group-based permissions - Add SSH key management UI (restricted to staff/instructors) - Fix require paths for services in controllers - Add services directory to autoload paths - Create bootstrap script for existing courses - Add host system user setup scripts - Support BusyBox useradd (use -p '*' instead of --disabled-password) - Add comprehensive setup and testing scripts

-              file.write(input_file.read)
-            end
+            dest = absolute_path.join(input_file.original_filename) 
+            File.open(dest, 'wb') { |f| f.write(input_file.read) }


General approach: ensure that all paths and filenames derived from user input are (a) constrained to live under BASE_DIRECTORY, and (b) sanitized so they cannot contain directory separators, excessive dots, or other dangerous characters. We will treat params[:path] as a logical path within BASE_DIRECTORY, normalize and validate it before calling safe_expand_path, and we will sanitize both params[:name] and input_file.original_filename before using them to create folders or files. This addresses both variants CodeQL reported.

Concrete fix in this file:

Add two helper methods in the private section:

sanitize_rel_path(path) which:

Forces the path into a relative, normalized form using Pathname#cleanpath.

Rejects any path that is absolute or that contains ".." segments.

sanitize_filename(name) which:

Uses a simple whitelist regex to restrict filenames to alphanumerics, underscore, dash, dot, and space.

Optionally collapses or strips leading dots and prevents blank results.

Use sanitize_rel_path in safe_expand_path so that path from user input is cleaned before joining with BASE_DIRECTORY. If sanitization fails, raise ActionController::RoutingError, 'Not Found' or ArgumentError.

Use sanitize_filename for:

params[:name] when creating a directory (dir = "#{absolute_path}/#{params[:name]}").

input_file.original_filename when calculating the destination file (dest = absolute_path.join(input_file.original_filename)).
Keep using the sanitized versions consistently for both existence checks and path creation, so the behavior (duplicate checking, etc.) is preserved but now based on safe names.

We only touch app/controllers/file_manager_controller.rb within the provided snippets, adding the helper methods in the private section and updating a few lines in upload_file and safe_expand_path. No new external gem is necessary; we can rely on Ruby’s Pathname and simple regexes.

This reverts commit 224f4ee.

This reverts commit 73326f8.

This reverts commit 18d9fa4.

Futurecomputerfixer and others added 30 commits October 21, 2025 13:02

implemented a file system permission cleaner and hooked it up in each…

31567a3

… web api function to enforce it

auto infer group name from path

73b747b

modify ssh

5308c40

fix to test on gamma

23ed6c2

testing

8e71819

add a new daemon container

55788af

fix bugs

214ebef

fix bugs ++

af54293

add hint for ssh key creation

1d31d75

add symlink

2551e51

testing ec2, add support on autolab side

325bae0

Filesystem mapping + EC2 autograder changes

aa12873

fix group

35bc7ec

fix groupa

266d150

fix groupa

2f3897b

fix groupa

0eca6f1

fix groupa

317feb1

fix groupa

d43ee75

fix groupa

fd4fc5c

fix groupa

9ccc99a

fix groupa

bdec585

fix groupa

48ec3a3

fix groupa

82addd1

add support for EC2 settings in Autolab

3e86a34

fix issue with autograder initialization

d43b4e1

fix groupa

20cba9e

fix groupa

4d45276

fix groupa

6d62851

fix groupa

25c4c05

Futurecomputerfixer and others added 6 commits December 17, 2025 20:19

fix groupa

40d398d

fix groupa

adf4012

fix groupa

65fd7d2

fix groupa

5910b83

fix groupa

9fbf5d3

editing ssh commands

f0be93f

KesterTan requested a review from jhs-panda February 2, 2026 21:55

added service user to classes

5bbdab8

github-advanced-security AI found potential problems Feb 6, 2026

View reviewed changes

attempt to fix course permissions

18d9fa4

github-advanced-security AI found potential problems Feb 9, 2026

View reviewed changes

Comment thread app/services/unix_group_manager.rb Fixed

KesterTan added 6 commits February 9, 2026 16:23

unix file system scanner

73326f8

install assessment

224f4ee

Revert "install assessment"

fbbd098

This reverts commit 224f4ee.

Revert "unix file system scanner"

ade67b8

This reverts commit 73326f8.

Revert "attempt to fix course permissions"

b0b6f90

This reverts commit 18d9fa4.

make user9999 always an instructor

4367517

KesterTan force-pushed the feature/filesystem-mapping branch from 60c2994 to 4367517 Compare February 16, 2026 06:13

KesterTan closed this Apr 15, 2026

@@ -151,27 +151,29 @@
                   if check_instructor(absolute_path) && !params[:name].nil?
                     all_filenames = Dir.entries(absolute_path)
                     if params[:name] != ""
-                      if all_filenames.include?(params[:name].to_s)
-                        raise "File with name #{input_file.original_filename} already exists."
+                      sanitized_dir_name = sanitize_filename(params[:name])
+                      if all_filenames.include?(sanitized_dir_name)
+                        raise "File or directory with name #{sanitized_dir_name} already exists."
                       end
                       # Creating a folder
-                      dir = "#{absolute_path}/#{params[:name]}"
+                      dir = absolute_path.join(sanitized_dir_name)
                       FileUtils.mkdir_p(dir)
-                      FilesystemEnforcer.fix_path(dir)
+                      FilesystemEnforcer.fix_path(dir.to_s)
                     else
                       # Uploading a file
                       input_file = params[:file]
                       return unless input_file
-                      if all_filenames.include?(input_file.original_filename)
-                        raise "File with name #{input_file.original_filename} already exists."
+                      sanitized_filename = sanitize_filename(input_file.original_filename)
+                      if all_filenames.include?(sanitized_filename)
+                        raise "File with name #{sanitized_filename} already exists."
                       elsif input_file.size >= 1.gigabyte
                         raise "File size is too large. Upload a file that is smaller than 1 GB."
                       else
-                        dest = absolute_path.join(input_file.original_filename)
+                        dest = absolute_path.join(sanitized_filename)
                         File.open(dest, 'wb') { |f| f.write(input_file.read) }
-                        FilesystemEnforcer.fix_path(dest.to_s)
+                        FilesystemEnforcer.fix_path(dest.to_s)
                       end
                     end
                   else
@@ -183,6 +172,33 @@
             private
+              def sanitize_rel_path(path)
+                rel = (path || "").to_s
+                # Normalize path (remove redundant ".", "..", and slashes)
+                clean = Pathname.new(rel).cleanpath.to_s
+                # Disallow absolute paths
+                if clean.start_with?(File::SEPARATOR) || clean.start_with?("\\")
+                  raise ActionController::RoutingError, 'Not Found'
+                end
+                # After normalization, reject any remaining parent-directory traversal
+                if clean.split(/[\\\/]/).include?("..")
+                  raise ActionController::RoutingError, 'Not Found'
+                end
+                clean
+              end
+              def sanitize_filename(name)
+                raw = (name || "").to_s
+                # Allow only a safe subset of characters in filenames
+                safe = raw.gsub(/[^0-9A-Za-z.\- _]/, "_")
+                # Prevent empty or dot-only names
+                safe = safe.sub(/\A\.+/, "")
+                if safe.empty?
+                  raise ActionController::RoutingError, 'Not Found'
+                end
+                safe
+              end
               def populate_directory(current_directory, current_url)
                 directory = Dir.entries(current_directory)
                 new_url = current_url == '/' ? '' : current_url
@@ -225,7 +241,9 @@
               def safe_expand_path(path)
                 current_directory = Pathname.new(BASE_DIRECTORY)
-                tested_path = Pathname.new(File.join(BASE_DIRECTORY, path))
+                # Sanitize the relative path coming from user input to prevent directory traversal
+                sanitized_relative = sanitize_rel_path(path)
+                tested_path = Pathname.new(File.join(BASE_DIRECTORY, sanitized_relative))
                 unless Archive.in_dir?(tested_path, current_directory, strict: false)
                   raise ArgumentError, 'Should not be parent of root'
                 end

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/filesystem mapping#2309

Feature/filesystem mapping#2309
KesterTan wants to merge 44 commits intomasterfrom
feature/filesystem-mapping

KesterTan commented Feb 2, 2026

Uh oh!

Check failure

Copilot Autofix

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

KesterTan commented Feb 2, 2026

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

Check failure

Uh oh!

Uh oh!

Copilot Autofix

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants