Add setup script and README to configure Brother HL-L1222(V) print server on Raspberry Pi (openSUSE)#32
Conversation
Reviewer's GuideAdds a new Bash setup script and documentation to turn a Raspberry Pi (openSUSE Tumbleweed) into a CUPS/IPP print server for a Brother HL‑L1222(V), including automated dependency installation, CUPS/firewalld/Avahi configuration, printer queue creation, a hotfolder auto‑print systemd service, and an optional LAN upload web service. Sequence diagram for setup script end-to-end configurationsequenceDiagram
actor User
participant Script as SetupScript
participant Zypper
participant Systemd
participant Firewalld
participant CUPS
participant Avahi as AvahiDaemon
participant USB as USBSubsystem
User->>Script: Run setup_hl_l1222_android_print.sh
Script->>Zypper: Install cups, avahi, firewalld, drivers, tools
Script->>Systemd: enable --now cups.service
Script->>Systemd: enable --now avahi-daemon.service
Script->>Systemd: enable --now firewalld.service
Script->>Systemd: enable --now bluetooth.service (optional)
Script->>Firewalld: Add ipp and mdns services
Script->>Firewalld: Add upload_port (optional)
Script->>Firewalld: Reload rules
Script->>User: Prompt to modify cupsd.conf
User-->>Script: Confirm
Script->>CUPS: cupsctl --share-printers --remote-any
Script->>CUPS: Append WebInterface, Browsing, BrowseLocalProtocols
Script->>Systemd: Restart cups.service
Script->>CUPS: lpinfo -v (detect USB)
CUPS->>USB: Enumerate USB printers
CUPS-->>Script: Device URI for Brother
Script->>CUPS: lpinfo -m (select model)
alt Existing queue
Script->>User: Ask to delete existing queue
User-->>Script: Confirm
Script->>CUPS: lpadmin -x QUEUE
end
Script->>CUPS: lpadmin -p QUEUE -E -v URI -m MODEL
Script->>CUPS: Set defaults, share, enable, accept
Script->>Systemd: Install and enable hll1222-hotfolder.service
Script->>Systemd: Install and enable hll1222-upload.service (optional)
Script-->>User: Print connection and usage summary
Sequence diagram for LAN upload to hotfolder printingsequenceDiagram
actor User
participant Browser
participant Upload as UploadService_hll1222-upload
participant Hotfolder as HotfolderDirectory
participant Watcher as HotfolderWatcher_hll1222-hotfolder
participant CUPS
participant Printer
User->>Browser: Open http://pi:UPLOAD_PORT/
Browser->>Upload: GET /
Upload-->>Browser: HTML upload form
User->>Browser: Select file and submit
Browser->>Upload: POST /upload (multipart file)
Upload->>Upload: Validate size and extension
Upload->>Hotfolder: Write file into hotfolder
Upload-->>Browser: Success response
Watcher->>Hotfolder: Monitor with inotifywait
Hotfolder-->>Watcher: Event close_write or moved_to
Watcher->>CUPS: lp -d QUEUE file
CUPS->>Printer: Send job via USB
Printer-->>User: Printed document
Class diagram for upload server HTTP handler and environmentclassDiagram
class H {
+send_body(code int, body str, ctype str)
+do_GET()
+do_POST()
}
class ThreadingHTTPServer {
}
class EnvironmentConfig {
+hot : Path
+queue : str
+maxb : int
+allowed : set
}
EnvironmentConfig <.. H : uses
ThreadingHTTPServer o-- H : handler
Flow diagram for hotfolder auto-print systemd serviceflowchart LR
subgraph System["Raspberry Pi"]
HF["Hotfolder /var/spool/hll1222-hotfolder"]
W["watch_print.sh (inotifywait loop)"]
SVC["Systemd service hll1222-hotfolder"]
CUPS["CUPS queue Brother_HL_L1222_V"]
end
P["USB Brother HL-L1222(V)"]
SVC --> W
W --> HF
HF -->|New file event| W
W -->|lp -d QUEUE file| CUPS
CUPS -->|Print job| P
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
--statusflag likely never works as intended becauseSTATUS_ONLYis declaredlocalinmain()but set inparse_args(), soparse_argsonly updates the global variable while theif [[ "${STATUS_ONLY:-no}" == "yes" ]]check reads the untouched local; consider using either a single global or passing the flag explicitly instead of mixing scopes. - In
print_summary, the lineecho " $LPSTAT_BIN -t"is fine but the next oneecho " "$LPINFO_BIN" -m | grep ..."has mismatched quoting and will be split by the shell; wrap the whole command string in one pair of quotes or use printf to avoid accidental word-splitting. - The hotfolder is created world-writable (
chmod 0777), which is convenient but quite permissive on a multi-user system; consider tightening permissions (e.g., a dedicated group plus setgid bit) so only trusted users and the upload service can write into it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `--status` flag likely never works as intended because `STATUS_ONLY` is declared `local` in `main()` but set in `parse_args()`, so `parse_args` only updates the global variable while the `if [[ "${STATUS_ONLY:-no}" == "yes" ]]` check reads the untouched local; consider using either a single global or passing the flag explicitly instead of mixing scopes.
- In `print_summary`, the line `echo " $LPSTAT_BIN -t"` is fine but the next one `echo " "$LPINFO_BIN" -m | grep ..."` has mismatched quoting and will be split by the shell; wrap the whole command string in one pair of quotes or use printf to avoid accidental word-splitting.
- The hotfolder is created world-writable (`chmod 0777`), which is convenient but quite permissive on a multi-user system; consider tightening permissions (e.g., a dedicated group plus setgid bit) so only trusted users and the upload service can write into it.
## Individual Comments
### Comment 1
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="501-510" />
<code_context>
+hot = Path(os.environ.get("PRINT_HOTFOLDER", "/var/spool/hll1222-hotfolder"))
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `item.raw` from `python-multipart` is likely incorrect and may break uploads.
`parse_form_data` usually returns a file-like object where content is read via `item.file.read()`, not a `.raw` attribute, so this will likely fail at runtime.
Use an explicit read from the file object and apply the same size limit to the bytes you actually write:
```python
data = item.file.read(maxb + 1)
if len(data) > maxb:
self.send_body(413, "Invalid upload size")
return
with open(dst, "wb") as f:
f.write(data)
```
This avoids relying on a non-standard attribute and keeps the size check consistent with the stored content.
</issue_to_address>
### Comment 2
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="391-399" />
<code_context>
+ err "Failed to create script directory ${SCRIPT_DIR}."
+ exit 1
+ fi
+ if ! sudo install -d -m 0775 -o lp -g lp "$HOTFOLDER"; then
+ err "Failed to create hotfolder ${HOTFOLDER}."
+ exit 1
+ fi
+ # Keep folder writable for upload service and watcher workflows.
+ if ! sudo chmod 0777 "$HOTFOLDER"; then
+ err "Failed to set writable permissions on ${HOTFOLDER}."
+ exit 1
</code_context>
<issue_to_address>
**🚨 suggestion (security):** World-writable hotfolder (`0777`) is risky; consider tightening permissions.
The directory is first created with `lp:lp` ownership and then broadened to `0777`, which exposes it to writes from any local user or compromised service. Given that the upload service runs as `lp:lp` and the watcher as `root`, it should be sufficient to keep it group-owned by `lp` and writable only to that user/group, for example:
```bash
sudo install -d -m 0770 -o lp -g lp "$HOTFOLDER"
```
If multiple writers are required, use a dedicated group instead of making the directory globally writable.
```suggestion
# Create hotfolder owned by lp:lp; writable by owner and group only.
# The upload service runs as lp:lp and the watcher runs as root, so 0770 is sufficient.
if ! sudo install -d -m 0770 -o lp -g lp "$HOTFOLDER"; then
err "Failed to create hotfolder ${HOTFOLDER}."
exit 1
fi
```
</issue_to_address>
### Comment 3
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="529-536" />
<code_context>
+ if "multipart/form-data" not in ctype:
+ self.send_body(400, "Content-Type must be multipart/form-data")
+ return
+ clen = int(self.headers.get("Content-Length", "0"))
+ if clen <= 0 or clen > maxb:
+ self.send_body(413, "Invalid upload size")
+ return
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Content-Length parsing can raise and bypass your validation logic.
`int(self.headers.get("Content-Length", "0"))` will raise `ValueError` if the header is missing or non-numeric, causing a 500 instead of your intended 4xx.
Consider guarding the conversion:
```python
try:
clen = int(self.headers.get("Content-Length", "0"))
except ValueError:
self.send_body(411, "Invalid or missing Content-Length")
return
if clen <= 0 or clen > maxb:
self.send_body(413, "Invalid upload size")
return
```
so malformed headers are handled cleanly instead of crashing the handler.
```suggestion
ctype = self.headers.get("Content-Type", "")
if "multipart/form-data" not in ctype:
self.send_body(400, "Content-Type must be multipart/form-data")
return
try:
clen = int(self.headers.get("Content-Length", "0"))
except ValueError:
self.send_body(411, "Invalid or missing Content-Length")
return
if clen <= 0 or clen > maxb:
self.send_body(413, "Invalid upload size")
return
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| hot = Path(os.environ.get("PRINT_HOTFOLDER", "/var/spool/hll1222-hotfolder")) | ||
| queue = os.environ.get("PRINT_QUEUE", "Brother_HL_L1222_V") | ||
| maxb = int(os.environ.get("MAX_UPLOAD_BYTES", "52428800")) | ||
| allowed = {".pdf", ".txt", ".png", ".jpg", ".jpeg", ".tif", ".tiff"} | ||
|
|
||
| hot.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| page = f"""<!doctype html><html><head><meta charset='utf-8'><meta name='viewport' content='width=device-width,initial-scale=1'><title>HL-L1222 Upload</title></head><body><h1>HL-L1222 Upload</h1><p>Queue: <b>{html.escape(queue)}</b></p><form method='post' enctype='multipart/form-data' action='/upload'><input type='file' name='file' required><button type='submit'>Upload</button></form><p>LAN only. Allowed: PDF/TXT/PNG/JPG/TIFF</p></body></html>""" | ||
|
|
||
| class H(BaseHTTPRequestHandler): |
There was a problem hiding this comment.
issue (bug_risk): Using item.raw from python-multipart is likely incorrect and may break uploads.
parse_form_data usually returns a file-like object where content is read via item.file.read(), not a .raw attribute, so this will likely fail at runtime.
Use an explicit read from the file object and apply the same size limit to the bytes you actually write:
data = item.file.read(maxb + 1)
if len(data) > maxb:
self.send_body(413, "Invalid upload size")
return
with open(dst, "wb") as f:
f.write(data)This avoids relying on a non-standard attribute and keeps the size check consistent with the stored content.
| if ! sudo install -d -m 0775 -o lp -g lp "$HOTFOLDER"; then | ||
| err "Failed to create hotfolder ${HOTFOLDER}." | ||
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then | ||
| err "Failed to set writable permissions on ${HOTFOLDER}." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🚨 suggestion (security): World-writable hotfolder (0777) is risky; consider tightening permissions.
The directory is first created with lp:lp ownership and then broadened to 0777, which exposes it to writes from any local user or compromised service. Given that the upload service runs as lp:lp and the watcher as root, it should be sufficient to keep it group-owned by lp and writable only to that user/group, for example:
sudo install -d -m 0770 -o lp -g lp "$HOTFOLDER"If multiple writers are required, use a dedicated group instead of making the directory globally writable.
| if ! sudo install -d -m 0775 -o lp -g lp "$HOTFOLDER"; then | |
| err "Failed to create hotfolder ${HOTFOLDER}." | |
| exit 1 | |
| fi | |
| # Keep folder writable for upload service and watcher workflows. | |
| if ! sudo chmod 0777 "$HOTFOLDER"; then | |
| err "Failed to set writable permissions on ${HOTFOLDER}." | |
| exit 1 | |
| fi | |
| # Create hotfolder owned by lp:lp; writable by owner and group only. | |
| # The upload service runs as lp:lp and the watcher runs as root, so 0770 is sufficient. | |
| if ! sudo install -d -m 0770 -o lp -g lp "$HOTFOLDER"; then | |
| err "Failed to create hotfolder ${HOTFOLDER}." | |
| exit 1 | |
| fi |
| ctype = self.headers.get("Content-Type", "") | ||
| if "multipart/form-data" not in ctype: | ||
| self.send_body(400, "Content-Type must be multipart/form-data") | ||
| return | ||
| clen = int(self.headers.get("Content-Length", "0")) | ||
| if clen <= 0 or clen > maxb: | ||
| self.send_body(413, "Invalid upload size") | ||
| return |
There was a problem hiding this comment.
suggestion (bug_risk): Content-Length parsing can raise and bypass your validation logic.
int(self.headers.get("Content-Length", "0")) will raise ValueError if the header is missing or non-numeric, causing a 500 instead of your intended 4xx.
Consider guarding the conversion:
try:
clen = int(self.headers.get("Content-Length", "0"))
except ValueError:
self.send_body(411, "Invalid or missing Content-Length")
return
if clen <= 0 or clen > maxb:
self.send_body(413, "Invalid upload size")
returnso malformed headers are handled cleanly instead of crashing the handler.
| ctype = self.headers.get("Content-Type", "") | |
| if "multipart/form-data" not in ctype: | |
| self.send_body(400, "Content-Type must be multipart/form-data") | |
| return | |
| clen = int(self.headers.get("Content-Length", "0")) | |
| if clen <= 0 or clen > maxb: | |
| self.send_body(413, "Invalid upload size") | |
| return | |
| ctype = self.headers.get("Content-Type", "") | |
| if "multipart/form-data" not in ctype: | |
| self.send_body(400, "Content-Type must be multipart/form-data") | |
| return | |
| try: | |
| clen = int(self.headers.get("Content-Length", "0")) | |
| except ValueError: | |
| self.send_body(411, "Invalid or missing Content-Length") | |
| return | |
| if clen <= 0 or clen > maxb: | |
| self.send_body(413, "Invalid upload size") | |
| return |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd749d112
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ! sudo zypper --non-interactive install --no-recommends \ | ||
| cups cups-client cups-filters ghostscript \ | ||
| avahi nss-mdns firewalld usbutils python3 python3-python-multipart; then | ||
| err "Failed to install required base packages." |
There was a problem hiding this comment.
The base dependency install always includes python3-python-multipart, even though the upload server is optional and disabled by default. On repos where that package is missing (which the script already anticipates later), install_dependencies exits early and the whole printer setup fails before reaching the normal IPP workflow. This should be gated behind ENABLE_UPLOAD (or treated as optional) so default installs are not blocked by an optional feature.
Useful? React with 👍 / 👎.
| lp -d "$QUEUE" -o fit-to-page "$file" || true | ||
| rm -f "$file" || true |
There was a problem hiding this comment.
The watcher deletes each file unconditionally right after lp, even when print submission fails (lp ... || true). If CUPS is down, the queue is paused, or lp rejects the job, documents are silently removed and unrecoverable, which is data loss for users relying on the hotfolder. Deletion should only occur after successful submission.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 README instructs sudo but script documents running as normal user
The README at line 34 instructs sudo bash scripts/setup_hl_l1222_android_print.sh, while the script's own header (line 15-16) says to run as a normal user with sudo used internally. Running the entire script as root means sudo calls are redundant (harmless) but lpoptions -d at line 373 would set the default printer for root only, not for the normal user who will use the printer. This inconsistency is worth aligning in documentation.
(Refers to line 34)
Was this helpful? React with 👍 or 👎 to provide feedback.
| safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160] | ||
| dst = hot / safe |
There was a problem hiding this comment.
🔴 Filename truncation can strip extension, causing uploaded files to silently never be printed
At line 556 the sanitized filename is truncated to 160 characters: safe = "...")[:160]. If the original filename exceeds 160 chars, the truncation can remove the file extension (e.g., .pdf). The file is then saved to the hotfolder at line 557 (dst = hot / safe) without its extension. The hotfolder watcher script (scripts/setup_hl_l1222_android_print.sh:419-420) only processes files matching *.pdf|*.jpg|*.jpeg|*.png|*.txt|*.tif|*.tiff, so the extensionless file is silently ignored — never printed and never cleaned up. Meanwhile, the user receives a "Uploaded successfully" response at line 565, creating a silent failure where the user believes the file was queued for printing but it never prints.
Note on the collision-avoidance path
The collision loop at lines 559-561 uses the original ext variable, so it would correctly append the extension — but only when a file with the same name already exists. On the first (non-colliding) upload, the truncated safe without extension is used directly.
| safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160] | |
| dst = hot / safe | |
| stem = ".".join(name.split(".")[:-1]) if "." in name else name | |
| safe_stem = ".".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in stem) | |
| safe = (safe_stem[:160 - len(ext)] + ext) if len(safe_stem) + len(ext) > 160 else safe_stem + ext | |
| safe = ".".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name) if len(safe) <= 160 else safe | |
| dst = hot / safe |
Was this helpful? React with 👍 or 👎 to provide feedback.
| exit 1 | ||
| fi | ||
|
|
||
| TMP_DIR="" |
There was a problem hiding this comment.
📝 Info: TMP_DIR is leaked on normal execution path
Both install_hotfolder_service (line 401) and install_upload_service_optional (line 488) create temp dirs via mktemp -d, but at lines 472 and 623 they set TMP_DIR="" rather than removing the temp dir. The cleanup trap (line 62-66) only runs on EXIT/SIGINT/SIGTERM, and by then TMP_DIR is empty, so the temp dirs are never removed. For a one-shot setup script this is a very minor resource leak, but for correctness the functions should rm -rf the temp dir before clearing the variable, or simply not clear it so the EXIT trap handles it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| --queue) | ||
| QUEUE_NAME="$2" | ||
| shift 2 | ||
| ;; | ||
| --device) | ||
| DEVICE_URI="$2" | ||
| shift 2 | ||
| ;; | ||
| --model) | ||
| MODEL_NAME="$2" | ||
| shift 2 | ||
| ;; | ||
| --enable-bluetooth) | ||
| ENABLE_BLUETOOTH="yes" | ||
| shift | ||
| ;; | ||
| --disable-bluetooth) | ||
| ENABLE_BLUETOOTH="no" | ||
| shift | ||
| ;; | ||
| --enable-upload) | ||
| ENABLE_UPLOAD="yes" | ||
| shift | ||
| ;; | ||
| --disable-upload) | ||
| ENABLE_UPLOAD="no" | ||
| shift | ||
| ;; | ||
| --upload-port) | ||
| UPLOAD_PORT="$2" | ||
| shift 2 | ||
| ;; |
There was a problem hiding this comment.
📝 Info: parse_args with set -u gives cryptic error on missing option arguments
Options like --queue, --device, --model, and --upload-port access $2 without checking that a second argument exists (e.g., lines 115, 119, 123, 143). With set -u (from set -euo pipefail at line 2), running ./script.sh --queue without a value triggers bash: $2: unbound variable — a valid but user-unfriendly error. This isn't a correctness bug since the script safely aborts, but adding a guard like [[ $# -ge 2 ]] || { err "--queue requires a value"; exit 1; } before accessing $2 would improve usability.
Was this helpful? React with 👍 or 👎 to provide feedback.
| local STATUS_ONLY="no" | ||
| parse_args "$@" |
There was a problem hiding this comment.
📝 Info: STATUS_ONLY variable scoping works correctly via bash dynamic scoping
main() declares local STATUS_ONLY="no" at line 657, then calls parse_args at line 658 which may set STATUS_ONLY="yes" at line 147. This works correctly because bash uses dynamic scoping — assignments in called functions modify the caller's local variable. I verified this is not a scoping bug: the --status flag will be properly detected at line 675.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if "multipart/form-data" not in ctype: | ||
| self.send_body(400, "Content-Type must be multipart/form-data") | ||
| return | ||
| clen = int(self.headers.get("Content-Length", "0")) |
There was a problem hiding this comment.
📝 Info: Upload server Content-Length parsing could raise ValueError on malformed input
At line 533 in the embedded Python server, clen = int(self.headers.get("Content-Length", "0")) would raise ValueError if a malicious client sends a non-numeric Content-Length header (e.g., Content-Length: abc). Since ThreadingHTTPServer handles each request in a separate thread, only the individual handler thread would crash — the server continues running. This is a minor robustness issue for a LAN-only service, but wrapping in a try/except would be more robust.
Was this helpful? React with 👍 or 👎 to provide feedback.
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then |
There was a problem hiding this comment.
📝 Info: Hotfolder has world-writable permissions (0777)
At line 396, the hotfolder is set to chmod 0777. The comment explains this is intentional for the upload service (running as lp user) and watcher (running as root). However, this means any local user on the system can drop files into the hotfolder to be printed. For a single-purpose Raspberry Pi this is acceptable, but on a shared system it could be an abuse vector (e.g., anyone can trigger unlimited print jobs). Consider using group-based permissions instead (e.g., 0775 with appropriate group membership).
Was this helpful? React with 👍 or 👎 to provide feedback.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive setup script and documentation for configuring a Raspberry Pi as a print server for Brother HL-L1222 printers on openSUSE Tumbleweed. The review feedback identifies several critical improvements: correcting the README to avoid unnecessary root execution, fixing a bug that causes temporary directory leaks by resetting the TMP_DIR variable, enhancing security by running services with reduced privileges and tightening file permissions, and improving the robustness of script argument parsing and command efficiency.
|
|
||
| ## Run | ||
| ```bash | ||
| sudo bash scripts/setup_hl_l1222_android_print.sh |
There was a problem hiding this comment.
The command sudo bash ... runs the entire script as root. However, the script is designed to be run as a normal user, and it uses sudo internally for privileged operations (as documented in scripts/setup_hl_l1222_android_print.sh on lines 15-16). Running the whole script as root is unnecessary, goes against the script's design, and could lead to unexpected behavior.
| sudo bash scripts/setup_hl_l1222_android_print.sh | |
| bash scripts/setup_hl_l1222_android_print.sh |
| cleanup() { | ||
| if [[ -n "${TMP_DIR}" && -d "${TMP_DIR}" ]]; then | ||
| rm -rf "${TMP_DIR}" | ||
| fi | ||
| } | ||
| trap cleanup EXIT SIGINT SIGTERM |
There was a problem hiding this comment.
The cleanup function and trap are correctly set up to remove temporary directories on exit. However, this is defeated because the TMP_DIR variable is reset to an empty string within the functions that use it (install_hotfolder_service at line 472 and install_upload_service_optional at line 623). This causes temporary directories created by mktemp to be leaked.
To fix this, please remove the TMP_DIR="" statements from line 472 and line 623.
| User=root | ||
| Group=root |
There was a problem hiding this comment.
Running the hotfolder watcher service as root poses a security risk, as it processes files from a directory that could be writable by other users. It's best practice to run services with the least privilege necessary. The lp user should have sufficient permissions for this task.
| User=root | |
| Group=root | |
| User=lp | |
| Group=lp |
| QUEUE_NAME="$2" | ||
| shift 2 |
There was a problem hiding this comment.
The argument parsing logic is not robust against missing option arguments. For example, if the script is called with --queue but no value, $2 will be the next option (e.g., --device), which is then incorrectly assigned to QUEUE_NAME. You should add a check to ensure an argument is provided and it's not another option. This should be applied to all options that require an argument.
| QUEUE_NAME="$2" | |
| shift 2 | |
| if [[ -z "$2" || "$2" =~ ^- ]]; then | |
| err "Option --queue requires an argument." | |
| usage | |
| exit 1 | |
| fi | |
| QUEUE_NAME="$2" | |
| shift 2 |
| fi | ||
|
|
||
| # 2) Fallback to everywhere if available. | ||
| if "$LPINFO_BIN" -m | awk '{print $1}' | grep -qx 'everywhere'; then |
There was a problem hiding this comment.
The pipeline of awk and grep to check for the 'everywhere' model is inefficient. It can be simplified into a single, more performant awk command that exits as soon as a match is found.
| if "$LPINFO_BIN" -m | awk '{print $1}' | grep -qx 'everywhere'; then | |
| if "$LPINFO_BIN" -m | awk '$1 == "everywhere" { exit 0 } END { exit 1 }'; then |
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then |
There was a problem hiding this comment.
Using chmod 0777 for the hotfolder is excessively permissive and a security risk, as it allows any user on the system to modify its contents. Given that the services using this folder run as lp or root, 0775 permissions would be more appropriate and secure.
| if ! sudo chmod 0777 "$HOTFOLDER"; then | |
| if ! sudo chmod 0775 "$HOTFOLDER"; then |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:29">
P3: The README incorrectly documents `everywhere` as the default model; the script actually prefers Brother/brlaser and only falls back to `everywhere`.</violation>
</file>
<file name="scripts/setup_hl_l1222_android_print.sh">
<violation number="1" location="scripts/setup_hl_l1222_android_print.sh:193">
P1: Don't make the optional upload dependency a hard requirement. Installing `python3-python-multipart` in the base package set can abort the entire setup even when upload support is disabled.</violation>
<violation number="2" location="scripts/setup_hl_l1222_android_print.sh:396">
P1: The hotfolder is set to `0777`, allowing any local user to inject or tamper with print jobs. Restrict it to the service account/group.</violation>
<violation number="3" location="scripts/setup_hl_l1222_android_print.sh:421">
P1: Only delete the hotfolder file after `lp` succeeds. The current flow removes files even when print submission fails, which can silently drop user documents.</violation>
<violation number="4" location="scripts/setup_hl_l1222_android_print.sh:445">
P1: The hotfolder watcher runs as root while processing user-supplied files. Run it as a non-root account (e.g., `lp`) to reduce privilege exposure.</violation>
<violation number="5" location="scripts/setup_hl_l1222_android_print.sh:533">
P2: Guard `Content-Length` parsing with `try/except` so malformed headers return a controlled 4xx response instead of raising in the request handler.</violation>
<violation number="6" location="scripts/setup_hl_l1222_android_print.sh:556">
P1: Preserve the original extension when truncating long filenames. Truncating the full name to 160 characters can strip `.pdf/.jpg/...`, causing the watcher to skip the uploaded file.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then |
There was a problem hiding this comment.
P1: The hotfolder is set to 0777, allowing any local user to inject or tamper with print jobs. Restrict it to the service account/group.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 396:
<comment>The hotfolder is set to `0777`, allowing any local user to inject or tamper with print jobs. Restrict it to the service account/group.</comment>
<file context>
@@ -0,0 +1,690 @@
+ exit 1
+ fi
+ # Keep folder writable for upload service and watcher workflows.
+ if ! sudo chmod 0777 "$HOTFOLDER"; then
+ err "Failed to set writable permissions on ${HOTFOLDER}."
+ exit 1
</file context>
|
|
||
| [Service] | ||
| Type=simple | ||
| User=root |
There was a problem hiding this comment.
P1: The hotfolder watcher runs as root while processing user-supplied files. Run it as a non-root account (e.g., lp) to reduce privilege exposure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 445:
<comment>The hotfolder watcher runs as root while processing user-supplied files. Run it as a non-root account (e.g., `lp`) to reduce privilege exposure.</comment>
<file context>
@@ -0,0 +1,690 @@
+
+[Service]
+Type=simple
+User=root
+Group=root
+Environment=PRINT_QUEUE=${QUEUE_NAME}
</file context>
| if ext not in allowed: | ||
| self.send_body(400, f"Unsupported file type: {html.escape(ext)}") | ||
| return | ||
| safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160] |
There was a problem hiding this comment.
P1: Preserve the original extension when truncating long filenames. Truncating the full name to 160 characters can strip .pdf/.jpg/..., causing the watcher to skip the uploaded file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 556:
<comment>Preserve the original extension when truncating long filenames. Truncating the full name to 160 characters can strip `.pdf/.jpg/...`, causing the watcher to skip the uploaded file.</comment>
<file context>
@@ -0,0 +1,690 @@
+ if ext not in allowed:
+ self.send_body(400, f"Unsupported file type: {html.escape(ext)}")
+ return
+ safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160]
+ dst = hot / safe
+ i = 1
</file context>
| inotifywait -m "$HOTFOLDER" -e close_write -e moved_to --format '%w%f' | while read -r file; do | ||
| case "${file,,}" in | ||
| *.pdf|*.jpg|*.jpeg|*.png|*.txt|*.tif|*.tiff) | ||
| lp -d "$QUEUE" -o fit-to-page "$file" || true |
There was a problem hiding this comment.
P1: Only delete the hotfolder file after lp succeeds. The current flow removes files even when print submission fails, which can silently drop user documents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 421:
<comment>Only delete the hotfolder file after `lp` succeeds. The current flow removes files even when print submission fails, which can silently drop user documents.</comment>
<file context>
@@ -0,0 +1,690 @@
+inotifywait -m "$HOTFOLDER" -e close_write -e moved_to --format '%w%f' | while read -r file; do
+ case "${file,,}" in
+ *.pdf|*.jpg|*.jpeg|*.png|*.txt|*.tif|*.tiff)
+ lp -d "$QUEUE" -o fit-to-page "$file" || true
+ rm -f "$file" || true
+ ;;
</file context>
| log "Installing required packages..." | ||
| if ! sudo zypper --non-interactive install --no-recommends \ | ||
| cups cups-client cups-filters ghostscript \ | ||
| avahi nss-mdns firewalld usbutils python3 python3-python-multipart; then |
There was a problem hiding this comment.
P1: Don't make the optional upload dependency a hard requirement. Installing python3-python-multipart in the base package set can abort the entire setup even when upload support is disabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 193:
<comment>Don't make the optional upload dependency a hard requirement. Installing `python3-python-multipart` in the base package set can abort the entire setup even when upload support is disabled.</comment>
<file context>
@@ -0,0 +1,690 @@
+ log "Installing required packages..."
+ if ! sudo zypper --non-interactive install --no-recommends \
+ cups cups-client cups-filters ghostscript \
+ avahi nss-mdns firewalld usbutils python3 python3-python-multipart; then
+ err "Failed to install required base packages."
+ exit 1
</file context>
| avahi nss-mdns firewalld usbutils python3 python3-python-multipart; then | |
| avahi nss-mdns firewalld usbutils python3; then |
| if "multipart/form-data" not in ctype: | ||
| self.send_body(400, "Content-Type must be multipart/form-data") | ||
| return | ||
| clen = int(self.headers.get("Content-Length", "0")) |
There was a problem hiding this comment.
P2: Guard Content-Length parsing with try/except so malformed headers return a controlled 4xx response instead of raising in the request handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 533:
<comment>Guard `Content-Length` parsing with `try/except` so malformed headers return a controlled 4xx response instead of raising in the request handler.</comment>
<file context>
@@ -0,0 +1,690 @@
+ if "multipart/form-data" not in ctype:
+ self.send_body(400, "Content-Type must be multipart/form-data")
+ return
+ clen = int(self.headers.get("Content-Length", "0"))
+ if clen <= 0 or clen > maxb:
+ self.send_body(413, "Invalid upload size")
</file context>
| - `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V` | ||
|
|
||
| ## Notes | ||
| - Script uses CUPS `everywhere` model as default for broad compatibility. |
There was a problem hiding this comment.
P3: The README incorrectly documents everywhere as the default model; the script actually prefers Brother/brlaser and only falls back to everywhere.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 29:
<comment>The README incorrectly documents `everywhere` as the default model; the script actually prefers Brother/brlaser and only falls back to `everywhere`.</comment>
<file context>
@@ -1,3 +1,30 @@
+ - `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V`
+
+## Notes
+- Script uses CUPS `everywhere` model as default for broad compatibility.
+- If you have an official Brother HL-L1222-V Linux PPD/driver package, install it and adjust queue model if desired.
</file context>
| - Script uses CUPS `everywhere` model as default for broad compatibility. | |
| - Script prefers a Brother/brlaser model when available, then falls back to CUPS `everywhere` (and generic models) for compatibility. |
2 participants
Motivation
Description
scripts/setup_hl_l1222_android_print.sh, a comprehensive installer that installs packages, enables services (cups,avahi-daemon,firewalld, optionalbluetooth), configures firewall rules, and updates CUPS sharing including a backup of/etc/cups/cupsd.confand user confirmation prompts.printer-driver-brlaseror fallback toeverywhere/generic), then create or replace the CUPS queue usinglpadminand related commands.inotifywaitplus a systemd unit at/etc/systemd/system/to auto-print files dropped into/var/spool/hll1222-hotfolder.upload_server.py) and systemd unit for simple browser uploads to the hotfolder, and add aREADME.mddescribing usage and the run commandsudo bash scripts/setup_hl_l1222_android_print.sh.Testing
Codex Task
Summary by Sourcery
Add a configurable setup script and documentation to turn a Raspberry Pi (openSUSE Tumbleweed) into a CUPS-based network print server for a USB-connected Brother HL-L1222/HL-L1222-V, including optional hotfolder and browser upload workflows.
New Features:
Enhancements:
Summary by cubic
Add a one-step setup script and README to turn a Raspberry Pi (openSUSE Tumbleweed) into an IPP/Mopria print server for a USB Brother HL‑L1222/HL‑L1222‑V. This enables Android printing over the same Wi‑Fi, with optional hotfolder and simple browser upload.
scripts/setup_hl_l1222_android_print.shto install dependencies and enable services (cups,avahi-daemon,firewalld, optionalbluetooth)./etc/cups/cupsd.confand confirmation prompts; auto-detect USB URI and create/replace queueBrother_HL_L1222_Vwith model fallback (printer-driver-brlaser→everywhere→ generic).hll1222-hotfolder) usinginotifywaitto print files dropped into/var/spool/hll1222-hotfolder.upload_server.py,hll1222-upload) to accept browser uploads into the hotfolder.Written for commit dcd749d. Summary will update on new commits. Review in cubic