vibecoded fixes#1
Conversation
…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 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 —
|
There was a problem hiding this comment.
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
~/.bashrcand introduce a baseline.bashrcfor 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.
| 'esac' \ | ||
| > /etc/profile.d/workshop-shell.sh && \ | ||
| chmod 644 /etc/profile.d/workshop-shell.sh | ||
|
|
There was a problem hiding this comment.
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.
| # 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 |
| 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" |
There was a problem hiding this comment.
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.
the result of claude and codex bouncing off each other