Skip to content

Add Dockerfile validation to task validate command #122

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.

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
142 changes: 140 additions & 2 deletions cmd/validate/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func run(name string) error {
return err
}

if err := isDockerfileValid(name); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -110,12 +114,19 @@ func areSecretsValid(name string) error {
// check if the license is valid
// the license must be valid
func IsLicenseValid(name string) error {
ctx := context.Background()
client := github.New()
server, err := readServerYaml(name)
if err != nil {
return err
}

// Skip validation for servers without source project
if server.Source.Project == "" {
fmt.Println("✅ License validation skipped (no source project)")
return nil
}

ctx := context.Background()
client := github.New()
repository, err := client.GetProjectRepository(ctx, server.Source.Project)
if err != nil {
return err
Expand Down Expand Up @@ -173,6 +184,133 @@ func isIconValid(name string) error {
return nil
}

func isDockerfileValid(name string) error {
server, err := readServerYaml(name)
if err != nil {
return err
}

// Skip validation for servers without source project
if server.Source.Project == "" {
fmt.Println("✅ Dockerfile validation skipped (no source project)")
return nil
}

// Determine Dockerfile path - default to "Dockerfile" if not specified
dockerfilePath := server.Source.Dockerfile
if dockerfilePath == "" {
dockerfilePath = "Dockerfile"
}

// Validate Dockerfile path format
if err := validateDockerfilePath(dockerfilePath); err != nil {
return fmt.Errorf("invalid Dockerfile path: %w", err)
}

// Skip GitHub API validation for local development or if GITHUB_TOKEN is not set
if os.Getenv("GITHUB_TOKEN") == "" {
fmt.Println("🛑 Dockerfile content validation skipped (no GITHUB_TOKEN)")
return nil
}

// Fetch and validate Dockerfile content from GitHub
ctx := context.Background()
client := github.NewFromServer(server)

// Construct full path including directory if specified
fullPath := dockerfilePath
if server.Source.Directory != "" {
fullPath = filepath.Join(server.Source.Directory, dockerfilePath)
}

content, err := client.GetFileContent(ctx, server.Source.Project, server.Source.Branch, fullPath)
if err != nil {
return fmt.Errorf("failed to fetch Dockerfile: %w", err)
}

// Validate Dockerfile content
if err := validateDockerfileContent(content); err != nil {
return fmt.Errorf("invalid Dockerfile content: %w", err)
}

fmt.Println("✅ Dockerfile is valid")
return nil
}

func validateDockerfilePath(path string) error {
// Must be relative path
if strings.HasPrefix(path, "/") {
return fmt.Errorf("Dockerfile path must be relative, not absolute")
}

// Should not contain directory traversal
if strings.Contains(path, "..") {
return fmt.Errorf("Dockerfile path should not contain directory traversal (..) components")
}

// Should be a valid filename
if strings.Contains(path, "\x00") {
return fmt.Errorf("Dockerfile path contains invalid characters")
}

return nil
}

func validateDockerfileContent(content string) error {
lines := strings.Split(content, "\n")

// Remove empty lines and comments for validation
var validLines []string
for _, line := range lines {
trimmed := strings.TrimSpace(line)
if trimmed != "" && !strings.HasPrefix(trimmed, "#") {
validLines = append(validLines, trimmed)
}
}

if len(validLines) == 0 {
return fmt.Errorf("Dockerfile is empty or contains only comments")
}

// First instruction must be FROM
firstLine := strings.ToUpper(strings.TrimSpace(validLines[0]))
if !strings.HasPrefix(firstLine, "FROM ") {
return fmt.Errorf("Dockerfile must start with FROM instruction")
}

// Check for basic security issues
for _, line := range validLines {
upperLine := strings.ToUpper(line)

// Check for potential security issues
if strings.Contains(upperLine, "PASSWORD=") ||
strings.Contains(upperLine, "SECRET=") ||
strings.Contains(upperLine, "API_KEY=") ||
strings.Contains(upperLine, "TOKEN=") {
return fmt.Errorf("Dockerfile should not contain hardcoded credentials")
}
}

// Should have an ENTRYPOINT or CMD instruction
hasEntrypoint := false
hasCmd := false
for _, line := range validLines {
upperLine := strings.ToUpper(strings.TrimSpace(line))
if strings.HasPrefix(upperLine, "ENTRYPOINT ") {
hasEntrypoint = true
}
if strings.HasPrefix(upperLine, "CMD ") {
hasCmd = true
}
}

if !hasEntrypoint && !hasCmd {
return fmt.Errorf("Dockerfile must contain either ENTRYPOINT or CMD instruction")
}

return nil
}

func readServerYaml(name string) (servers.Server, error) {
serverYaml, err := os.ReadFile(filepath.Join("servers", name, "server.yaml"))
if err != nil {
Expand Down
98 changes: 98 additions & 0 deletions cmd/validate/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,101 @@ func Test_areSecretsValid(t *testing.T) {
})
}
}

func TestValidateDockerfilePath(t *testing.T) {
tests := []struct {
path string
wantErr bool
desc string
}{
{"Dockerfile", false, "simple Dockerfile"},
{"src/postgres/Dockerfile", false, "relative path with subdirectory"},
{"Dockerfile.local", false, "Dockerfile with suffix"},
{"/Dockerfile", true, "absolute path should be rejected"},
{"../Dockerfile", true, "directory traversal should be rejected"},
{"src/../Dockerfile", true, "directory traversal in middle should be rejected"},
{"Dockerfile\x00", true, "null byte should be rejected"},
}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := validateDockerfilePath(tt.path)
if (err != nil) != tt.wantErr {
t.Errorf("validateDockerfilePath(%q) error = %v, wantErr %v", tt.path, err, tt.wantErr)
}
})
}
}

func TestValidateDockerfileContent(t *testing.T) {
tests := []struct {
content string
wantErr bool
desc string
}{
{
content: "FROM node:18-alpine\nWORKDIR /app\nCMD [\"node\", \"index.js\"]",
wantErr: false,
desc: "valid simple Dockerfile",
},
{
content: "FROM alpine\nRUN apk add --no-cache curl\nENTRYPOINT [\"curl\"]",
wantErr: false,
desc: "valid Dockerfile with ENTRYPOINT",
},
{
content: "# Comment only Dockerfile\n# Another comment",
wantErr: true,
desc: "Dockerfile with only comments should fail",
},
{
content: "",
wantErr: true,
desc: "empty Dockerfile should fail",
},
{
content: "RUN echo hello\nFROM alpine",
wantErr: true,
desc: "Dockerfile not starting with FROM should fail",
},
{
content: "FROM node:18\nWORKDIR /app",
wantErr: true,
desc: "Dockerfile without CMD or ENTRYPOINT should fail",
},
{
content: "FROM alpine\nENV PASSWORD=secret123\nCMD [\"sh\"]",
wantErr: true,
desc: "Dockerfile with hardcoded password should fail",
},
{
content: "FROM alpine\nENV API_KEY=abc123\nCMD [\"sh\"]",
wantErr: true,
desc: "Dockerfile with hardcoded API key should fail",
},
{
content: "FROM alpine\nENV SECRET=mysecret\nENTRYPOINT [\"sh\"]",
wantErr: true,
desc: "Dockerfile with hardcoded secret should fail",
},
{
content: "FROM alpine\nENV TOKEN=mytoken\nCMD [\"sh\"]",
wantErr: true,
desc: "Dockerfile with hardcoded token should fail",
},
{
content: "FROM node:18\n# This is a comment\nWORKDIR /app\n# Another comment\nCMD [\"node\", \"app.js\"]",
wantErr: false,
desc: "valid Dockerfile with comments should pass",
},
}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := validateDockerfileContent(tt.content)
if (err != nil) != tt.wantErr {
t.Errorf("validateDockerfileContent() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
39 changes: 39 additions & 0 deletions pkg/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,45 @@ func (c *Client) FindIcon(ctx context.Context, projectURL string) (string, error
return repository.Owner.GetAvatarURL(), nil
}

func (c *Client) GetFileContent(ctx context.Context, projectURL, branch, filePath string) (string, error) {
owner, repo, err := extractOrgAndProject(projectURL)
if err != nil {
return "", err
}

if branch == "" {
repository, err := c.GetProjectRepository(ctx, projectURL)
if err != nil {
return "", err
}
branch = repository.GetDefaultBranch()
}

for {
fileContent, _, _, err := c.gh.Repositories.GetContents(ctx, owner, repo, filePath, &github.RepositoryContentGetOptions{
Ref: branch,
})
if sleepOnRateLimitError(ctx, err) {
continue
}

if err != nil {
return "", err
}

if fileContent == nil {
return "", fmt.Errorf("file not found: %s", filePath)
}

content, err := fileContent.GetContent()
if err != nil {
return "", err
}

return content, nil
}
}

func sleepOnRateLimitError(ctx context.Context, err error) bool {
var rateLimitErr *github.RateLimitError
if !errors.As(err, &rateLimitErr) {
Expand Down
35 changes: 35 additions & 0 deletions servers/rook-ceph-mcp-server/server.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: rook-ceph-mcp-server
image: mcp/rook-ceph-mcp-server
type: server
metadata:
category: Infrastructure
tags:
- kubernetes
- storage
- ceph
- rook
- infrastructure
- devops
about:
title: Rook Ceph MCP Server
description: A Model Context Protocol server for managing Rook Ceph storage clusters in Kubernetes environments. Provides tools for cluster management, storage resource operations, and pre-configured YAML templates.
icon: https://avatars.githubusercontent.com/u/35940573?s=200&v=4
source:
project: https://github.com/shreyanshjain7174/rook-ceph-mcp
config:
variables:
- name: KUBECONFIG
description: Path to Kubernetes configuration file
required: false
default: ~/.kube/config
- name: PORT
description: HTTP server port (when using HTTP transport)
required: false
default: "3000"
- name: NODE_ENV
description: Node.js environment (development/production)
required: false
default: production
secrets: []
dockerfile: Dockerfile
entry_point: ["npm", "run", "start:stdio"]