Skip to content

fix(middleware): use world-readable permissions for payload files #773

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
lpcox merged 1 commit into from
Feb 7, 2026
Merged

fix(middleware): use world-readable permissions for payload files #773

Show file tree
Hide file tree
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
8 changes: 4 additions & 4 deletions internal/middleware/jqschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@ func savePayload(baseDir, sessionID, queryID string, payload []byte) (string, er
logger.LogDebug("payload", "Creating payload directory: baseDir=%s, session=%s, query=%s, fullPath=%s",
baseDir, sessionID, queryID, dir)

if err := os.MkdirAll(dir, 0700); err != nil {
if err := os.MkdirAll(dir, 0755); err != nil {
logger.LogError("payload", "Failed to create payload directory: path=%s, error=%v", dir, err)
Comment on lines +132 to 133
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

os.MkdirAll only applies the provided mode when the directory is created; if the directory already exists its permissions are not updated. If this change is intended to make existing payload dirs readable by agents after an upgrade, add an explicit os.Chmod (and potentially for parent/session dirs) to ensure the effective permissions become 0755.

This issue also appears on line 132 of the same file.

Copilot uses AI. Check for mistakes.
return "", fmt.Errorf("failed to create payload directory: %w", err)
}

logger.LogDebug("payload", "Successfully created payload directory: path=%s, permissions=0700", dir)
logger.LogDebug("payload", "Successfully created payload directory: path=%s, permissions=0755", dir)

// Save payload to file with restrictive permissions (owner read/write only)
filePath := filepath.Join(dir, "payload.json")
Expand All @@ -143,13 +143,13 @@ func savePayload(baseDir, sessionID, queryID string, payload []byte) (string, er
logger.LogInfo("payload", "Writing large payload to filesystem: path=%s, size=%d bytes (%.2f KB, %.2f MB)",
filePath, payloadSize, float64(payloadSize)/1024, float64(payloadSize)/(1024*1024))

if err := os.WriteFile(filePath, payload, 0600); err != nil {
if err := os.WriteFile(filePath, payload, 0644); err != nil {
logger.LogError("payload", "Failed to write payload file: path=%s, size=%d bytes, error=%v",
filePath, payloadSize, err)
return "", fmt.Errorf("failed to write payload file: %w", err)
}

Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

os.WriteFile does not change permissions if payload.json already exists (it truncates but retains the existing mode). To guarantee agents can read old files created as 0600, follow the write with an explicit os.Chmod(filePath, 0644) (or open with Chmod on existing files) and handle any error.

This issue also appears on line 139 of the same file.

Suggested change
// Ensure file has the expected permissions, even if it already existed
if err := os.Chmod(filePath, 0644); err != nil {
logger.LogError("payload", "Failed to set payload file permissions: path=%s, permissions=%04o, error=%v",
filePath, 0644, err)
return "", fmt.Errorf("failed to set payload file permissions: %w", err)
}

Copilot uses AI. Check for mistakes.
logger.LogInfo("payload", "Successfully saved large payload to filesystem: path=%s, size=%d bytes, permissions=0600",
logger.LogInfo("payload", "Successfully saved large payload to filesystem: path=%s, size=%d bytes, permissions=0644",
filePath, payloadSize)

// Verify file was written correctly
Expand Down
4 changes: 2 additions & 2 deletions internal/middleware/jqschema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,12 +496,12 @@ func TestPayloadStorage_FilePermissions(t *testing.T) {
dirPath := filepath.Dir(filePath)
dirInfo, err := os.Stat(dirPath)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0700), dirInfo.Mode().Perm(), "Directory should have 0700 permissions")
assert.Equal(t, os.FileMode(0755), dirInfo.Mode().Perm(), "Directory should have 0755 permissions")

// Check file permissions
fileInfo, err := os.Stat(filePath)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0600), fileInfo.Mode().Perm(), "File should have 0600 permissions")
assert.Equal(t, os.FileMode(0644), fileInfo.Mode().Perm(), "File should have 0644 permissions")
}

// TestPayloadStorage_DefaultSessionID verifies behavior when session ID is empty
Expand Down
4 changes: 2 additions & 2 deletions internal/server/unified.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,8 @@ func (us *UnifiedServer) ensureSessionDirectory(sessionID string) error {
return fmt.Errorf("failed to check session directory: %w", err)
}

// Directory doesn't exist, create it with restrictive permissions (owner-only access)
if err := os.MkdirAll(sessionDir, 0700); err != nil {
// Directory doesn't exist, create it with world-readable permissions (for agent access)
if err := os.MkdirAll(sessionDir, 0755); err != nil {
return fmt.Errorf("failed to create session directory: %w", err)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/server/unified_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,8 @@ func TestUnifiedServer_EnsureSessionDirectory(t *testing.T) {
require.NoError(t, err, "Session directory should exist")
assert.True(t, info.IsDir(), "Session path should be a directory")

// Verify directory has correct permissions (0700)
assert.Equal(t, os.FileMode(0700), info.Mode().Perm(), "Session directory should have 0700 permissions")
// Verify directory has correct permissions (0755 - world-readable for agent access)
assert.Equal(t, os.FileMode(0755), info.Mode().Perm(), "Session directory should have 0755 permissions")

// Test that calling ensureSessionDirectory again doesn't fail (idempotent)
err = us.ensureSessionDirectory(sessionID)
Expand Down
Loading