Skip to content

Commit 899f1b4

Browse files
authored
fix: secure CORS defaults (#2387)
1 parent 9f92b01 commit 899f1b4

9 files changed

Lines changed: 592 additions & 20 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ The embedded API is experimental and may change. See the embedded API documentat
788788
| `DAGU_LOG_FORMAT` | `text` | `text` or `json` |
789789
| `DAGU_CERT_FILE` | — | TLS certificate |
790790
| `DAGU_KEY_FILE` | — | TLS private key |
791-
| `DAGU_CORS_ALLOWED_ORIGINS` | — | Comma-separated list of allowed CORS origins (e.g. `https://app.example.com`). When unset, all origins are allowed without credentials. When set, only listed origins are allowed and credentials are enabled. |
791+
| `DAGU_CORS_ALLOWED_ORIGINS` | — | Comma-separated list of allowed CORS origins (e.g. `https://app.example.com`). When unset, cross-origin browser access is disabled. Exact origins enable credentials. An explicit `*` allows every origin without credentials and emits a security warning. |
792792

793793
### Paths
794794

internal/cmn/config/config.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,9 @@ type Server struct {
150150
RemoteNodes []RemoteNode
151151
Permissions map[Permission]bool
152152
StrictValidation bool
153-
// CORSAllowedOrigins lists explicit origins for CORS. When empty, all
154-
// origins are allowed but AllowCredentials is disabled (spec-compliant).
155-
// When set, only listed origins are allowed and AllowCredentials is enabled.
153+
// CORSAllowedOrigins lists origins allowed to make cross-origin requests.
154+
// An empty list disables CORS. A literal wildcard explicitly allows every
155+
// origin without credentials; exact origins enable credentials.
156156
CORSAllowedOrigins []string
157157
Metrics MetricsAccess // "private" or "public"
158158
Terminal TerminalConfig

internal/cmn/config/loader.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,17 @@ func (l *ConfigLoader) loadServerDefaults(cfg *Config, def Definition) {
654654
cfg.Server.BasePath = cleanServerBasePath(cfg.Server.BasePath)
655655
cfg.Server.CheckUpdates = l.v.GetBool("check_updates")
656656
cfg.Server.CORSAllowedOrigins = parseStringList(l.v.Get("cors_allowed_origins"))
657+
for _, origin := range cfg.Server.CORSAllowedOrigins {
658+
if strings.TrimSpace(origin) != "*" {
659+
continue
660+
}
661+
warning := `cors_allowed_origins contains "*"; any website may make browser requests to the Dagu API`
662+
if cfg.Server.Auth.Mode == AuthModeNone {
663+
warning += `, and auth.mode "none" allows those requests to execute workflows without authentication`
664+
}
665+
l.warnings = append(l.warnings, warning)
666+
break
667+
}
657668

658669
cfg.Server.Metrics = MetricsAccessPrivate
659670
if def.Metrics != nil {

internal/cmn/config/loader_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,55 @@ metrics: "invalid_value"
16071607
})
16081608
}
16091609

1610+
func TestLoad_CORSAllowedOrigins(t *testing.T) {
1611+
t.Run("EmptyDisablesCORS", func(t *testing.T) {
1612+
cfg := loadFromYAML(t, `
1613+
auth:
1614+
mode: none
1615+
cors_allowed_origins: []
1616+
`)
1617+
assert.Empty(t, cfg.Server.CORSAllowedOrigins)
1618+
assert.Empty(t, cfg.Warnings)
1619+
})
1620+
1621+
t.Run("ExplicitOrigins", func(t *testing.T) {
1622+
cfg := loadFromYAML(t, `
1623+
auth:
1624+
mode: none
1625+
cors_allowed_origins:
1626+
- https://app.example.com
1627+
- https://admin.example.com
1628+
`)
1629+
assert.Equal(t, []string{"https://app.example.com", "https://admin.example.com"}, cfg.Server.CORSAllowedOrigins)
1630+
assert.Empty(t, cfg.Warnings)
1631+
})
1632+
1633+
t.Run("WildcardWarning", func(t *testing.T) {
1634+
cfg := loadFromYAML(t, `
1635+
auth:
1636+
mode: builtin
1637+
cors_allowed_origins:
1638+
- "*"
1639+
`)
1640+
assert.Equal(t, []string{"*"}, cfg.Server.CORSAllowedOrigins)
1641+
require.Len(t, cfg.Warnings, 1)
1642+
assert.Contains(t, cfg.Warnings[0], "any website may make browser requests")
1643+
})
1644+
1645+
t.Run("WildcardWithoutAuthWarning", func(t *testing.T) {
1646+
cfg := loadFromYAML(t, `
1647+
auth:
1648+
mode: none
1649+
cors_allowed_origins:
1650+
- "*"
1651+
`)
1652+
assert.Equal(t, []string{"*"}, cfg.Server.CORSAllowedOrigins)
1653+
require.Len(t, cfg.Warnings, 1)
1654+
assert.Contains(t, cfg.Warnings[0], `auth.mode "none"`)
1655+
assert.Contains(t, cfg.Warnings[0], "execute workflows without authentication")
1656+
})
1657+
}
1658+
16101659
func TestLoad_AccessLogMode(t *testing.T) {
16111660
t.Run("AccessLogAll", func(t *testing.T) {
16121661
cfg := loadFromYAML(t, `

internal/cmn/schema/config.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
"cors_allowed_origins": {
4646
"type": "array",
4747
"items": { "type": "string" },
48-
"description": "Explicit list of origins allowed to make cross-origin requests. When unset, all origins are allowed without credentials. When set, only listed origins are allowed and credentials are enabled. A wildcard '*' in the list is treated the same as unset."
48+
"description": "Origins allowed to make cross-origin requests. When unset or empty, cross-origin browser access is disabled. Exact origins enable credentials. An explicit '*' allows every origin without credentials and emits a security warning."
4949
},
5050
"debug": {
5151
"type": "boolean",
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (C) 2026 Yota Hamada
2+
// SPDX-License-Identifier: GPL-3.0-or-later
3+
4+
package api_test
5+
6+
import (
7+
"fmt"
8+
"net/http"
9+
"testing"
10+
11+
"github.com/dagucloud/dagu/internal/cmn/config"
12+
"github.com/dagucloud/dagu/internal/test"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestCORS_DefaultAndExplicitWildcard(t *testing.T) {
18+
t.Run("default denies cross-origin API preflight", func(t *testing.T) {
19+
server := test.SetupServer(t, test.WithConfigMutator(func(cfg *config.Config) {
20+
cfg.Server.Auth.Mode = config.AuthModeNone
21+
cfg.Server.CORSAllowedOrigins = nil
22+
}))
23+
24+
resp := sendPreflight(t, server, "/api/v1/dag-runs", "https://evil.example")
25+
defer func() { _ = resp.Body.Close() }()
26+
27+
assert.Equal(t, http.StatusForbidden, resp.StatusCode)
28+
assert.Empty(t, resp.Header.Get("Access-Control-Allow-Origin"))
29+
})
30+
31+
t.Run("explicit wildcard allows API but not setup", func(t *testing.T) {
32+
server := test.SetupServer(t, test.WithConfigMutator(func(cfg *config.Config) {
33+
cfg.Server.Auth.Mode = config.AuthModeNone
34+
cfg.Server.CORSAllowedOrigins = []string{"*"}
35+
}))
36+
37+
apiResp := sendPreflight(t, server, "/api/v1/dag-runs", "https://app.example")
38+
defer func() { _ = apiResp.Body.Close() }()
39+
require.Equal(t, http.StatusOK, apiResp.StatusCode)
40+
assert.Equal(t, "*", apiResp.Header.Get("Access-Control-Allow-Origin"))
41+
assert.Empty(t, apiResp.Header.Get("Access-Control-Allow-Credentials"))
42+
43+
setupResp := sendPreflight(t, server, "/api/v1/auth/setup", "https://app.example")
44+
defer func() { _ = setupResp.Body.Close() }()
45+
assert.Equal(t, http.StatusForbidden, setupResp.StatusCode)
46+
assert.Empty(t, setupResp.Header.Get("Access-Control-Allow-Origin"))
47+
})
48+
}
49+
50+
func sendPreflight(t *testing.T, server test.Server, requestPath, origin string) *http.Response {
51+
t.Helper()
52+
53+
requestURL := fmt.Sprintf(
54+
"http://%s:%d%s",
55+
server.Config.Server.Host,
56+
server.Config.Server.Port,
57+
requestPath,
58+
)
59+
req, err := http.NewRequest(http.MethodOptions, requestURL, nil)
60+
require.NoError(t, err)
61+
req.Header.Set("Origin", origin)
62+
req.Header.Set("Access-Control-Request-Method", http.MethodPost)
63+
req.Header.Set("Access-Control-Request-Headers", "content-type")
64+
65+
resp, err := http.DefaultClient.Do(req)
66+
require.NoError(t, err)
67+
return resp
68+
}

internal/service/frontend/cors.go

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Copyright (C) 2026 Yota Hamada
2+
// SPDX-License-Identifier: GPL-3.0-or-later
3+
4+
package frontend
5+
6+
import (
7+
"net"
8+
"net/http"
9+
"net/url"
10+
"strings"
11+
12+
"github.com/go-chi/cors"
13+
)
14+
15+
type corsPolicy struct {
16+
allowedOrigins []string
17+
publicURL string
18+
setupPath string
19+
}
20+
21+
func (p corsPolicy) middleware(next http.Handler) http.Handler {
22+
corsConfigured := len(p.allowedOrigins) > 0
23+
wrapped := next
24+
if corsConfigured {
25+
allowAllOrigins := p.allowsAllOrigins()
26+
options := cors.Options{
27+
AllowedMethods: []string{"GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"},
28+
AllowedHeaders: []string{"Content-Type", "Authorization", "Content-Encoding", "Accept", "MCP-Protocol-Version", "Mcp-Session-Id", "Last-Event-ID"},
29+
ExposedHeaders: []string{"Mcp-Session-Id"},
30+
AllowCredentials: !allowAllOrigins,
31+
MaxAge: 300,
32+
}
33+
if allowAllOrigins {
34+
options.AllowedOrigins = []string{"*"}
35+
} else {
36+
options.AllowOriginFunc = func(_ *http.Request, origin string) bool {
37+
return p.allowsOrigin(origin)
38+
}
39+
}
40+
wrapped = cors.Handler(options)(next)
41+
}
42+
43+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
44+
if !corsConfigured {
45+
w.Header().Add("Vary", "Origin")
46+
}
47+
origin := r.Header.Get("Origin")
48+
if origin != "" && p.isCrossOrigin(r, origin) {
49+
if p.isSetupPath(r.URL.Path) || !p.allowsOrigin(origin) {
50+
if corsConfigured {
51+
w.Header().Add("Vary", "Origin")
52+
}
53+
http.Error(w, "cross-origin request denied", http.StatusForbidden)
54+
return
55+
}
56+
}
57+
wrapped.ServeHTTP(w, r)
58+
})
59+
}
60+
61+
func (p corsPolicy) isCrossOrigin(r *http.Request, origin string) bool {
62+
sourceOrigin := canonicalOrigin(origin)
63+
if sourceOrigin == "" {
64+
return true
65+
}
66+
if sourceOrigin == requestOrigin(r) || sourceOrigin == canonicalOrigin(p.publicURL) {
67+
return false
68+
}
69+
70+
// Fetch Metadata preserves same-origin classification through reverse proxies
71+
// that do not expose the public scheme and host to the application.
72+
return !strings.EqualFold(strings.TrimSpace(r.Header.Get("Sec-Fetch-Site")), "same-origin")
73+
}
74+
75+
func requestOrigin(r *http.Request) string {
76+
scheme := "http"
77+
if r.TLS != nil {
78+
scheme = "https"
79+
}
80+
return canonicalOrigin(scheme + "://" + r.Host)
81+
}
82+
83+
func (p corsPolicy) allowsOrigin(origin string) bool {
84+
origin = strings.ToLower(strings.TrimSpace(origin))
85+
canonicalRequestOrigin := canonicalOrigin(origin)
86+
for _, candidate := range p.allowedOrigins {
87+
candidate = strings.ToLower(strings.TrimSpace(candidate))
88+
if candidate == "*" {
89+
return true
90+
}
91+
if prefix, suffix, ok := strings.Cut(candidate, "*"); ok {
92+
if len(origin) >= len(prefix)+len(suffix) &&
93+
strings.HasPrefix(origin, prefix) && strings.HasSuffix(origin, suffix) {
94+
return true
95+
}
96+
continue
97+
}
98+
if candidate == origin ||
99+
(canonicalRequestOrigin != "" && canonicalOrigin(candidate) == canonicalRequestOrigin) {
100+
return true
101+
}
102+
}
103+
return false
104+
}
105+
106+
func (p corsPolicy) allowsAllOrigins() bool {
107+
for _, origin := range p.allowedOrigins {
108+
if strings.TrimSpace(origin) == "*" {
109+
return true
110+
}
111+
}
112+
return false
113+
}
114+
115+
func (p corsPolicy) isSetupPath(requestPath string) bool {
116+
return strings.TrimRight(requestPath, "/") == strings.TrimRight(p.setupPath, "/")
117+
}
118+
119+
func canonicalOrigin(value string) string {
120+
parsed, err := url.Parse(strings.TrimSpace(value))
121+
if err != nil || parsed.Scheme == "" || parsed.Host == "" || parsed.User != nil {
122+
return ""
123+
}
124+
if !strings.EqualFold(parsed.Scheme, "http") && !strings.EqualFold(parsed.Scheme, "https") {
125+
return ""
126+
}
127+
128+
scheme := strings.ToLower(parsed.Scheme)
129+
hostname := strings.ToLower(parsed.Hostname())
130+
if hostname == "" {
131+
return ""
132+
}
133+
port := parsed.Port()
134+
if (scheme == "http" && port == "80") || (scheme == "https" && port == "443") {
135+
port = ""
136+
}
137+
138+
host := hostname
139+
if port != "" {
140+
host = net.JoinHostPort(hostname, port)
141+
} else if strings.Contains(hostname, ":") {
142+
host = "[" + hostname + "]"
143+
}
144+
return scheme + "://" + host
145+
}

0 commit comments

Comments
 (0)