Skip to content

fix(authz): Enforce per-deployment scope on container and resource actions#121

Merged
nfebe merged 1 commit into
mainfrom
fix/issue-74-authz-scope
May 9, 2026
Merged

fix(authz): Enforce per-deployment scope on container and resource actions#121
nfebe merged 1 commit into
mainfrom
fix/issue-74-authz-scope

Conversation

@nfebe
Copy link
Copy Markdown
Contributor

@nfebe nfebe commented May 9, 2026

Operators with read-only access to a deployment could previously perform write actions on its containers (SSH, lifecycle, resources) and see or mutate domains, SSL, backups, scheduler tasks, security events, traffic logs, and virtual hosts belonging to deployments they had no access to. Each protected action now checks the actor's access level for the target deployment, list endpoints filter results to the actor's accessible deployments, and global proxy sync is restricted to admins. WebSocket container exec rejects the session before signaling auth success when the actor lacks write access. API-key usage on the WebSocket path now updates last-used metadata for audit parity with HTTP requests.

Resolves #74

…tions

Operators with read-only access to a deployment could previously perform
write actions on its containers (SSH, lifecycle, resources) and see or
mutate domains, SSL, backups, scheduler tasks, security events, traffic
logs, and virtual hosts belonging to deployments they had no access to.
Each protected action now checks the actor's access level for the
target deployment, list endpoints filter results to the actor's
accessible deployments, and global proxy sync is restricted to admins.
WebSocket container exec rejects the session before signaling auth
success when the actor lacks write access. API-key usage on the
WebSocket path now updates last-used metadata for audit parity with
HTTP requests.
@sourceant
Copy link
Copy Markdown

sourceant Bot commented May 9, 2026

Code Review Summary

This PR implements robust per-deployment authorization across the API. It ensures that operators are restricted to resources (containers, backups, logs) belonging to deployments they have explicit access to.

🚀 Key Improvements

  • Mandatory deployment filtering for non-admin users on global list endpoints (Security Events, Traffic Logs).
  • Per-container authorization based on the com.docker.compose.project label.
  • WebSocket authentication now populates the actor context, enabling audit logging and permission checks for exec sessions.

💡 Minor Suggestions

  • Consider caching Docker label lookups to improve API response times.
  • The containerDeploymentName helper could use a more robust error handling for cases where the Docker daemon is unreachable.

Copy link
Copy Markdown

@sourceant sourceant Bot left a comment

Choose a reason for hiding this comment

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

Review complete. See the overview comment for a summary.

Comment thread internal/api/authz.go
if deploymentName == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "Deployment name required"})
return false
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Invoking a new process (docker inspect) on every authorization check is extremely inefficient and will lead to poor API performance. This should be replaced with a cache or an in-memory lookup from a previously populated map, similar to how listContainerDeploymentLabels is used in the stats module.

Suggested change
}
func (s *Server) getContainerDeployment(containerID string) (string, error) {
// TODO: Implement caching for container-to-deployment mapping
cmd := exec.Command("docker", "inspect", "--format", "{{ index .Config.Labels \""+composeProjectLabel+"\" }}", containerID)
output, err := cmd.Output()
if err != nil {
return "", err
}
deploymentName := strings.TrimSpace(string(output))
if deploymentName == "<no value>" {
return "", nil
}
return deploymentName, nil
}

return
}

actor := auth.GetActorFromContext(c)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Filtering the slice in-place using backups[:0] is efficient, but ensure that the underlying backups slice isn't used elsewhere after this function as the backing array is modified.

Suggested change
actor := auth.GetActorFromContext(c)
actor := auth.GetActorFromContext(c)
if actor != nil && actor.Role != auth.RoleAdmin {
filtered := make([]models.Backup, 0)
for _, b := range backups {
if actor.CanAccessDeployment(b.DeploymentName, auth.AccessLevelRead) {
filtered = append(filtered, b)
}
}
backups = filtered
}

@nfebe nfebe merged commit 7657a19 into main May 9, 2026
5 checks passed
@nfebe nfebe deleted the fix/issue-74-authz-scope branch May 9, 2026 09:44
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.

security: Several actions available to users that should not have permission to do

1 participant