Skip to content

vibecoded fixes#1

Closed
omaribrahim6 wants to merge 3 commits into
mainfrom
fixes
Closed

vibecoded fixes#1
omaribrahim6 wants to merge 3 commits into
mainfrom
fixes

Conversation

@omaribrahim6
Copy link
Copy Markdown
Collaborator

the result of claude and codex bouncing off each other

omaribrahim6 and others added 3 commits March 24, 2026 23:38
…nce, path normalization

- verifier 1.2/3.3: add command-log fallback for noatime hosts; atime check
  remains primary but now falls back to checking whether the student ran
  any command referencing the target file since the last reset.
  Also add set_log_marker to reset_1_2 and reset_3_3 so the fallback is
  scoped to the current attempt.
- verifier 7.2: broaden PATH pattern from export-only to also accept inline
  assignment (PATH=.../path/bin ...), so students using either form pass.
- verifier sorted_normalized_file_paths: skip malformed lines instead of
  aborting the whole function, so one bad line in recent.txt doesn't produce
  a false "file not found" error.
- install-host-broker.sh: create workshop-iptables.service (After=docker,
  reads subnet from broker.env) so egress block survives reboots.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 25, 2026 03:57
@omaribrahim6
Copy link
Copy Markdown
Collaborator Author

@copilot give a detailed summary of everything this branch is "fixing"

Copy link
Copy Markdown

Copilot AI commented Mar 25, 2026

@copilot give a detailed summary of everything this branch is "fixing"

Here's a detailed breakdown of everything this branch changes across the 3 commits:


Commit 1 — dbb4e9c "vibecoded fixes"

user-container/Dockerfile

  • Removed SSH server entirely. openssh-server is no longer installed, the SSH sshd_config.d/workshop.conf is no longer created, and EXPOSE 22 is removed. The container no longer runs sshd.
  • Command logging moved out of ~/.bashrc. Previously the _log_cmd hook was injected into the student's ~/.bashrc, which could be broken by challenge 7.3 (which edits .bashrc). Now it lives in /etc/profile.d/workshop-shell.sh so students can't accidentally break it. The function was also renamed _workshop_log_cmd and duplicates are guarded with a PROMPT_COMMAND idempotency check.
  • Saved a clean .bashrc baseline to /opt/.bashrc.baseline so reset_7_3 can fully restore .bashrc instead of just removing one line.

user-container/entrypoint.sh

  • Added set -euo pipefail for strict error handling.
  • Removed all SSH plumbing: SSH_SECRET_ENV_CONFIG variable, write_secret_env_config(), and configure_ssh_access() functions are all gone.
  • write_secure_token fixed: was using tee (which writes to stdout and file simultaneously), now uses a local variable so the token is written cleanly to the file.
  • Added start_flag_service(): wraps the python3 flag-server.py launch with a liveness check — if the process dies within 0.2 s, the entrypoint exits with an error instead of silently continuing.
  • Non-interactive mode changed: instead of exec-ing sshd -D -e as the container's main process, it now runs exec tail -f /dev/null (the container stays alive via a no-op sleep loop rather than an SSH daemon).

user-container/verifier.sh

  • Removed SSH_SECRET_ENV_CONFIG and write_secret_env_config(); added BASHRC_BASELINE constant.
  • write_secure_token fix (same as entrypoint: tee → local variable; now also prints the token back to stdout for callers that capture it).
  • Added normalize_path() — wraps readlink -f with an empty-input guard.
  • Added sorted_normalized_file_paths() — reads a file of paths, strips Windows-style CR, makes relative paths absolute, resolves them with normalize_path, deduplicates, and returns them sorted. Used by verify_5_3.
  • Added worker_secret_process_running() — scans /proc/*/cmdline and /proc/*/environ to verify the worker process with the matching secret is actually running. Replaces the old worker_secret_running() function.
  • verify_1_2 simplified: old logic found the most-recently accessed file across the whole labyrinth and required it to be flag.txt. New logic just checks whether flag.txt's own atime has been updated — passes as long as the student read that specific file.
  • verify_3_1 strengthened: now actually runs show_flag.sh as the ieee user and checks its output is EXECUTE_SUCCESS, not just that the executable bit is set.
  • verify_3_3 simplified: mirrors the 1.2 change — checks the atime of the 600-permission file directly instead of requiring it to be the last file read across all audit files.
  • verify_5_1: reads only the first line of ~/bigfile.txt (strips trailing whitespace) and compares both sides with normalize_path so minor path formatting differences don't cause a false failure.
  • verify_5_2 relaxed: previously required the student to save the full grep -r output (with filename: prefix). Now accepts either the full filename:matching-line form or just the bare matching line.
  • verify_5_3: student's ~/recent.txt is now passed through sorted_normalized_file_paths before comparing, so relative paths and extra whitespace are tolerated.
  • verify_6_3: calls renamed function worker_secret_process_running.
  • reset_7_1: removed write_secret_env_config call (SSH is gone).
  • reset_7_3: now restores .bashrc from the saved baseline (/opt/.bashrc.baseline) instead of just deleting the alias hello= line.

Commit 2 — a31335d "Fix remaining issues: atime fallback, PATH pattern, iptables persistence, path normalization"

user-container/verifier.sh

  • sorted_normalized_file_paths fix: a bad line (e.g. a path that doesn't exist) used to return 1 and abort the whole function. Now it uses continue to skip the bad line, so one invalid entry doesn't cause a false failure.
  • verify_1_2 and verify_3_3 — command-log fallback for noatime mounts: on Linux hosts that mount with noatime (common in CI/containers), stat -c %X never updates, so atime-based detection always fails. Both verifiers now fall back to checking the command log: if the student ran a read command referencing the target file since the last reset, they pass even if atime wasn't updated.
  • reset_1_2 and reset_3_3: call set_log_marker so the command-log fallback is scoped to the current attempt (commands from before the reset don't count).
  • verify_7_2 — broader PATH pattern: the old regex only matched export PATH=.../path/bin. It now also matches inline assignment syntax (PATH=.../path/bin command) so students using either form get credit.

web-server/install-host-broker.sh

  • Adds workshop-iptables.service: a systemd oneshot service that re-applies the workshop egress firewall rules on every boot. Previously the iptables rules were applied once at install time and lost on reboot. The service reads the subnet from /etc/workshop/broker.env, runs after docker.service (so the DOCKER-USER chain exists), and is enabled via systemctl enable.

Commit 3 — 2d21e9f "more fixes from codex"

user-container/verifier.sh

  • verify_1_2 and verify_3_3 — tightened command-log fallback regex: the simple flag\.txt / $target_name pattern could be triggered by commands that mention the filename without actually reading it (e.g. rm flag.txt). Replaced with a more precise regex that only matches if the filename appears as an argument to a known read command (cat, less, more, head, tail, grep, sed, awk) or as a standalone argument, accounting for pipes, semicolons, and subshells.

web-server/uninstall-host-broker.sh

  • Cleans up workshop-iptables.service on uninstall: stops and disables the service, removes the unit file, reloads systemd, and removes the /usr/local/lib/workshop/workshop-iptables.sh helper script.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the workshop host broker installation to persist its iptables egress rules across reboots, and refactors the user-container runtime/verifier setup (notably removing the in-container SSH server and hardening several verifier checks).

Changes:

  • Add a dedicated workshop-iptables.service + helper script to re-apply WORKSHOP-EGRESS iptables rules after reboot (and uninstall it cleanly).
  • Update the user container to run without sshd, improve token writing behavior, and make several verifier checks less brittle.
  • Move command logging hook out of the student’s ~/.bashrc and introduce a baseline .bashrc for challenge resets.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web-server/install-host-broker.sh Adds persistent iptables rule application via systemd service.
web-server/uninstall-host-broker.sh Removes the new iptables systemd unit and helper script during uninstall.
user-container/verifier.sh Improves token handling, path normalization helpers, /proc secret verification, and log-marker usage.
user-container/entrypoint.sh Removes sshd flow; adds stricter shell options and a flag-service startup helper.
user-container/Dockerfile Removes openssh-server; moves command logging to /etc/profile.d and adds a .bashrc baseline.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread user-container/Dockerfile
'esac' \
> /etc/profile.d/workshop-shell.sh && \
chmod 644 /etc/profile.d/workshop-shell.sh

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

Command logging is now installed in /etc/profile.d/workshop-shell.sh, which only runs for login shells. If a student opens a subshell with bash (non-login interactive), their commands won't be logged, and logged_since-based verifier checks can fail unexpectedly. Consider also installing the PROMPT_COMMAND hook into /etc/bash.bashrc (or another file sourced by all interactive bash shells), or ensure login shells source it for non-login shells as well.

Suggested change
# Ensure command logging is also enabled in non-login interactive bash shells
RUN printf '%s\n' \
'if [ -f /etc/profile.d/workshop-shell.sh ]; then' \
' . /etc/profile.d/workshop-shell.sh' \
'fi' \
>> /etc/bash.bashrc

Copilot uses AI. Check for mistakes.
Comment on lines 603 to 613
verify_5_1() {
EXPECTED=$(cat "$BIGFILE_PATH_FILE" 2>/dev/null)
ACTUAL=$(cat "$HOME/bigfile.txt" 2>/dev/null | tr -d '[:space:]')
ACTUAL=$(sed -n '1{s/[[:space:]]*$//;p;}' "$HOME/bigfile.txt" 2>/dev/null)
if [ -z "$EXPECTED" ]; then
fail "Challenge state is missing the large file path."
hint "Run 'reset 5.1' to restore the challenge state, then try again."
elif [ -z "$ACTUAL" ]; then
fail "File ~/bigfile.txt not found or is empty."
hint "Use 'find' with a size filter, then save the result to ~/bigfile.txt."
elif [ "$ACTUAL" = "$EXPECTED" ]; then
elif [ "$(normalize_path "$ACTUAL")" = "$(normalize_path "$EXPECTED")" ]; then
pass "5.1" "big_file_found"
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

verify_5_1 normalizes the path using readlink -f, but if ~/bigfile.txt contains a relative path (e.g. output from find .), it will be resolved relative to the current working directory at verify time, making verification flaky depending on where the user runs verify. Consider treating non-absolute paths as relative to $HOME (similar to sorted_normalized_file_paths) or explicitly rejecting relative paths with a clearer failure message.

Copilot uses AI. Check for mistakes.
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.

3 participants