Skip to content

Potential fix for code scanning alert no. 1: Uncontrolled command line #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karam-ajaj merged 1 commit into from
Jan 2, 2026
Merged

Potential fix for code scanning alert no. 1: Uncontrolled command line #82

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion config/scripts/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,20 @@ def validate_container_name(name: str) -> str:
raise HTTPException(status_code=400, detail="Invalid container name format")
return name

def validate_log_filename(name: str) -> str:
"""
Validate a log filename so it can be safely used to construct a path and
passed as an argument to subprocess calls.
"""
if not name or len(name) > 255:
raise HTTPException(status_code=400, detail="Invalid log filename length")
# Disallow any path separators and restrict to a safe character set
if "/" in name or "\\" in name:
raise HTTPException(status_code=400, detail="Invalid log filename format")
if not re.fullmatch(r"[a-zA-Z0-9._-]+", name):
raise HTTPException(status_code=400, detail="Invalid log filename format")
Comment on lines +268 to +269
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

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

The validation regex allows dots in filenames, which could potentially be used for path traversal via sequences like .. (parent directory). While the os.path.normpath and os.path.commonpath checks on lines 296-298 provide defense in depth, it would be safer to either disallow consecutive dots or add explicit validation to reject names containing .. sequences before the path construction step. This would provide earlier detection of malicious input.

Copilot uses AI. Check for mistakes.
return name

@app.get("/logs/container/{container_name}", tags=["Docker"])
def get_container_logs(container_name: str):
try:
Expand All @@ -277,8 +291,9 @@ def event_generator():
safe_container = validate_container_name(container)
cmd = ["docker", "logs", "-f", "--tail", "10", safe_container]
else:
safe_filename = validate_log_filename(filename)
base_dir = os.path.abspath(LOGS_DIR)
filepath = os.path.normpath(os.path.join(base_dir, filename))
filepath = os.path.normpath(os.path.join(base_dir, safe_filename))
# Ensure the resolved path stays within the logs directory
if os.path.commonpath([base_dir, filepath]) != base_dir:
yield "data: [ERROR] Invalid log file path\n\n"
Expand Down