Skip to content
Closed
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
42 changes: 40 additions & 2 deletions apps/api/internal/handler/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
// Allowed instance setting section keys (must match migration seed).
var allowedSettingKeys = map[string]bool{
"general": true, "email": true, "auth": true, "oauth": true, "ai": true, "image": true,
"github_app": true,
"github_app": true, "slack_app": true,
}

// InstanceHandler serves instance setup (first-run); no auth required.
Expand Down Expand Up @@ -269,7 +269,7 @@ func (h *InstanceSettingsHandler) GetSettings(c *gin.Context) {
out[k] = decryptSectionSecretsInternal(k, row.Value)
}
// Ensure all sections exist with defaults (migration seed may not have run if DB was created before seed)
for _, key := range []string{"general", "email", "auth", "oauth", "ai", "image", "github_app"} {
for _, key := range []string{"general", "email", "auth", "oauth", "ai", "image", "github_app", "slack_app"} {
if _, ok := out[key]; !ok {
out[key] = defaultSettingValue(key)
}
Expand All @@ -284,6 +284,7 @@ var secretKeysBySection = map[string][]string{
"ai": {"api_key"},
"image": {"unsplash_access_key"},
"github_app": {"private_key", "client_secret", "webhook_secret"},
"slack_app": {"client_secret", "signing_secret"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not serialize decrypted Slack secrets.

Registering these fields makes both settings GET and PATCH responses return plaintext Slack credentials via decryptSectionSecretsInternal, despite the UI contract that secrets are never echoed. Redact secret fields from API responses and return only their *_set flags.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/internal/handler/instance.go` at line 287, Update the Slack secret
registration near the "slack_app" configuration so decrypted client_secret and
signing_secret are excluded from settings GET and PATCH responses. Preserve only
the corresponding client_secret_set and signing_secret_set flags in the API
response, consistent with the UI contract that plaintext secrets are never
returned.

}

// decryptSectionSecretsInternal returns a copy of m with secret fields decrypted.
Expand Down Expand Up @@ -328,6 +329,8 @@ func defaultSettingValue(key string) model.JSONMap {
"app_id": "", "app_name": "", "client_id": "",
"client_secret_set": false, "private_key_set": false, "webhook_secret_set": false,
}
case "slack_app":
return model.JSONMap{"client_id": "", "client_secret_set": false, "signing_secret_set": false}
default:
return model.JSONMap{}
}
Expand Down Expand Up @@ -517,6 +520,41 @@ func (h *InstanceSettingsHandler) UpdateSetting(c *gin.Context) {
setSecret("webhook_secret", "webhook_secret_set")
value = merged
}
if key == "slack_app" {
existing, _ := h.Settings.Get(c.Request.Context(), "slack_app")
merged := model.JSONMap{}

if existing != nil {
for k, v := range existing.Value {
merged[k] = v
}
} else {
for k, v := range defaultSettingValue("slack_app") {
merged[k] = v
}
}

/* plain feilds */
for _, feild := range []string{"client_id"} {
if v, ok := req.Value[feild]; ok {
merged[feild] = v
}
}

/* Secret fields (Encrypt & Set flag) */
setSecret := func(feild, setKey string) {
if v, ok := req.Value[feild]; ok {
if s, ok := v.(string); ok && s != "" {
merged[feild] = crypto.EncryptOrPlain(s)
merged[setKey] = true
}
}
}

setSecret("client_secret", "client_secret_set")
setSecret("signing_secret", "signing_secret_set")
value = merged
}
Comment on lines +523 to +557

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Keep Slack settings updates in the service layer.

This new handler branch calls h.Settings.Get directly. Move Slack merge/encryption logic behind an instance-settings service so the handler delegates and the service owns store access.

As per path instructions, apps/api/**/*.go requires “handlers call services, services call stores.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/internal/handler/instance.go` around lines 523 - 557, Move the
Slack-specific merge and secret-encryption logic from the handler branch around
the key "slack_app" into an instance-settings service method, including the
h.Settings.Get access. Update the handler to delegate the Slack settings update
to that service and use its returned value, preserving default merging,
client_id handling, and secret flags.

Source: Path instructions

if err := h.Settings.Upsert(c.Request.Context(), key, value); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save settings"})
return
Expand Down
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"type": "module",
"scripts": {
"dev": "vite",
"start": "vite",
"build": "tsc -b && vite build",
"typecheck": "tsc -b --noEmit",
"lint": "eslint --max-warnings=0 .",
Expand Down
9 changes: 9 additions & 0 deletions apps/web/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,15 @@ export interface InstanceGitHubAppSection {
webhook_secret_set?: boolean;
}

/* Slack App config (instance admin). Secrets are never echoed back. */
export interface InstanceSlackAppSection {
client_id?: string;
client_secret?: string;
client_secret_set?: boolean;
signing_secret?: string;
signing_secret_set?: boolean;
}

/** Available integration provider, returned by GET /api/integrations/. */
export interface IntegrationApiResponse {
id: string;
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/components/layout/InstanceAdminLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ const AUTH_SUB_LABEL: Record<string, string> = {

const INTEGRATIONS_SUB_LABEL: Record<string, string> = {
github: 'GitHub',
slack: 'Slack',
};

export function InstanceAdminLayout() {
Expand Down
Loading
Loading