feat: allow relative target paths for git mounts
- Remove absolute path requirement from target_path validation - Resolve relative paths against working_directory at instance startup - Fall back to /home/user if no working_directory is configured - Update frontend to allow relative target paths - Update spec to document relative path support - Update tests to allow relative paths and test path traversal rejection
This commit is contained in:
@@ -82,8 +82,6 @@ class GitMountItem(BaseModel):
|
|||||||
@field_validator("target_path")
|
@field_validator("target_path")
|
||||||
@classmethod
|
@classmethod
|
||||||
def validate_target_path(cls, v: str) -> str:
|
def validate_target_path(cls, v: str) -> str:
|
||||||
if not v.startswith("/"):
|
|
||||||
raise ValueError("target_path must be absolute (start with /)")
|
|
||||||
if ".." in v:
|
if ".." in v:
|
||||||
raise ValueError("target_path cannot contain path traversal (..)")
|
raise ValueError("target_path cannot contain path traversal (..)")
|
||||||
return v
|
return v
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ async def _resolve_git_mounts(
|
|||||||
session: AsyncSession,
|
session: AsyncSession,
|
||||||
resolved: ResolvedProfile,
|
resolved: ResolvedProfile,
|
||||||
instance_dir: str | None = None,
|
instance_dir: str | None = None,
|
||||||
|
working_directory: str | None = None,
|
||||||
) -> list[dict]:
|
) -> list[dict]:
|
||||||
"""Convert git mounts from resolved profile to Docker volume mounts.
|
"""Convert git mounts from resolved profile to Docker volume mounts.
|
||||||
|
|
||||||
@@ -74,7 +75,7 @@ async def _resolve_git_mounts(
|
|||||||
# Process all git mounts concurrently
|
# Process all git mounts concurrently
|
||||||
tasks = []
|
tasks = []
|
||||||
for git_mount in resolved.git_mounts:
|
for git_mount in resolved.git_mounts:
|
||||||
tasks.append(_resolve_single_git_mount(session, git_mount, instance_dir))
|
tasks.append(_resolve_single_git_mount(session, git_mount, instance_dir, working_directory))
|
||||||
|
|
||||||
results = await asyncio.gather(*tasks, return_exceptions=True)
|
results = await asyncio.gather(*tasks, return_exceptions=True)
|
||||||
|
|
||||||
@@ -93,6 +94,7 @@ async def _resolve_single_git_mount(
|
|||||||
session: AsyncSession,
|
session: AsyncSession,
|
||||||
git_mount: dict,
|
git_mount: dict,
|
||||||
instance_dir: str | None = None,
|
instance_dir: str | None = None,
|
||||||
|
working_directory: str | None = None,
|
||||||
) -> list[dict]:
|
) -> list[dict]:
|
||||||
"""Resolve a single git mount to volume mount entries.
|
"""Resolve a single git mount to volume mount entries.
|
||||||
|
|
||||||
@@ -108,6 +110,12 @@ async def _resolve_single_git_mount(
|
|||||||
logger.warning("Invalid git mount skipped: missing remote_url or target_path")
|
logger.warning("Invalid git mount skipped: missing remote_url or target_path")
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
# Resolve relative target paths against working directory
|
||||||
|
if target_path and not target_path.startswith("/"):
|
||||||
|
base = working_directory or "/home/user"
|
||||||
|
target_path = os.path.join(base, target_path)
|
||||||
|
logger.info("Resolved relative target path to %s", target_path)
|
||||||
|
|
||||||
if not instance_dir:
|
if not instance_dir:
|
||||||
logger.warning("Git mount skipped: no instance_dir provided for cloning")
|
logger.warning("Git mount skipped: no instance_dir provided for cloning")
|
||||||
return []
|
return []
|
||||||
@@ -918,7 +926,7 @@ async def start_instance(
|
|||||||
# Profile mounts are added to extra volumes
|
# Profile mounts are added to extra volumes
|
||||||
extra_volumes.extend(profile_mounts)
|
extra_volumes.extend(profile_mounts)
|
||||||
# Git repository mounts are resolved and added
|
# Git repository mounts are resolved and added
|
||||||
git_mount_volumes = await _resolve_git_mounts(session, resolved, instance_dir)
|
git_mount_volumes = await _resolve_git_mounts(session, resolved, instance_dir, working_directory)
|
||||||
extra_volumes.extend(git_mount_volumes)
|
extra_volumes.extend(git_mount_volumes)
|
||||||
# Profile runtime hints override tool config values
|
# Profile runtime hints override tool config values
|
||||||
if profile_hints.get("start_command"):
|
if profile_hints.get("start_command"):
|
||||||
|
|||||||
@@ -402,8 +402,8 @@ class TestConfigProfilesAPI:
|
|||||||
)
|
)
|
||||||
assert response.status_code == 422
|
assert response.status_code == 422
|
||||||
|
|
||||||
def test_create_config_profile_invalid_git_mount_target_path(self, authenticated_client: TestClient, test_project_and_repo) -> None:
|
def test_create_config_profile_invalid_git_mount_target_path_traversal(self, authenticated_client: TestClient, test_project_and_repo) -> None:
|
||||||
"""Test that invalid git mount target paths are rejected."""
|
"""Test that git mount target paths with traversal are rejected."""
|
||||||
_project_id, repo_id = test_project_and_repo
|
_project_id, repo_id = test_project_and_repo
|
||||||
|
|
||||||
response = authenticated_client.post(
|
response = authenticated_client.post(
|
||||||
@@ -416,7 +416,7 @@ class TestConfigProfilesAPI:
|
|||||||
{
|
{
|
||||||
"remote_url": "https://github.com/user/repo.git",
|
"remote_url": "https://github.com/user/repo.git",
|
||||||
"source_path": ".",
|
"source_path": ".",
|
||||||
"target_path": "relative/path",
|
"target_path": "../../../etc/passwd",
|
||||||
}
|
}
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -36,7 +36,6 @@ export const GitMountEditor = ({ mounts, onChange }: GitMountEditorProps) => {
|
|||||||
const validatePath = (path: string, isTarget: boolean): string | null => {
|
const validatePath = (path: string, isTarget: boolean): string | null => {
|
||||||
if (!path) return isTarget ? "Target path is required" : null;
|
if (!path) return isTarget ? "Target path is required" : null;
|
||||||
if (path.includes("..")) return "Path cannot contain ..";
|
if (path.includes("..")) return "Path cannot contain ..";
|
||||||
if (isTarget && !path.startsWith("/")) return "Target path must be absolute";
|
|
||||||
if (!isTarget && path.startsWith("/")) return "Source path must be relative";
|
if (!isTarget && path.startsWith("/")) return "Source path must be relative";
|
||||||
return null;
|
return null;
|
||||||
};
|
};
|
||||||
@@ -200,7 +199,7 @@ const GitMountForm = ({ mount, onSave, onCancel, validatePath, validateUrl, isNe
|
|||||||
placeholder="e.g., /app/config"
|
placeholder="e.g., /app/config"
|
||||||
className={errors.target_path ? "error" : ""}
|
className={errors.target_path ? "error" : ""}
|
||||||
/>
|
/>
|
||||||
<span className="hint">Absolute path inside container</span>
|
<span className="hint">Absolute or relative path inside container (relative resolved against working dir)</span>
|
||||||
{errors.target_path && <span className="error-text">{errors.target_path}</span>}
|
{errors.target_path && <span className="error-text">{errors.target_path}</span>}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ The system SHALL allow config profiles to include git repository mounts that bin
|
|||||||
- **THEN** the system validates that:
|
- **THEN** the system validates that:
|
||||||
- `remote_url` is a valid git URL (starts with https://, git@, or ssh://)
|
- `remote_url` is a valid git URL (starts with https://, git@, or ssh://)
|
||||||
- `source_path` is a relative path (no leading `/`)
|
- `source_path` is a relative path (no leading `/`)
|
||||||
- `target_path` is an absolute path (starts with `/`)
|
- `target_path` can be absolute (starts with `/`) or relative (resolved against working directory, defaulting to `/home/user`)
|
||||||
- `target_path` does not contain path traversal sequences (`..`)
|
- `target_path` does not contain path traversal sequences (`..`)
|
||||||
- No database lookup or repository existence check is performed (validation is deferred to clone time)
|
- No database lookup or repository existence check is performed (validation is deferred to clone time)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user