opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: add --chat-hook-allow-insecure to allow plain HTTP chat hook URLs (#27896)

Browse Source 
Adds a hidden `--chat-hook-allow-insecure` /
`CODER_CHAT_HOOK_ALLOW_INSECURE` deployment option (default `false`)
that allows the chat lifecycle hook URL to use plain HTTP for any host.

The HTTPS requirement is enforced at two points, and the flag relaxes
both: `DeploymentValues.Validate()` rejects `http` hook URLs at startup,
and the hook dispatcher's `validateHookURL` allows `http` only for
loopback hosts. With the flag set, any-host `http` is accepted; the
host, fragment/userinfo, secret, and timeout checks are unchanged, and
non-http(s) schemes still fail. This removes the need for an HTTPS
reverse proxy when testing a hook consumer on a trusted network.

Following security review feedback, the flag description and docs state
that plain HTTP lets an on-path attacker forge hook responses (which
control agent execution), and `coder server` logs a startup warning
(with a redacted hook URL) when hooks run over plain HTTP.

Docs, generated API types, and the server config golden are updated
accordingly.

> Mux acted on Mike's behalf to create this PR.
This commit is contained in:
Michael Suchacz
2026-08-05 22:41:17 +02:00
committed by GitHub
parent 3e2a8bd421
commit 4b9880afa6
16 changed files with 161 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+3
View File
@@ -17407,6 +17407,9 @@ const docTemplate = `{
"debug_logging_enabled": {
"type": "boolean"
},
"hook_allow_insecure": {
"type": "boolean"
},
"hook_enabled": {
"type": "boolean"
},
coderd/apidoc/swagger.json
Generated
+3
View File
@@ -15644,6 +15644,9 @@
"debug_logging_enabled": {
"type": "boolean"
},
"hook_allow_insecure": {
"type": "boolean"
},
"hook_enabled": {
"type": "boolean"
},
coderd/coderd.go
+6
View File
@@ -894,10 +894,16 @@ func New(options *Options) *API {
)
}
if hooksConfigured && hooksExperimentEnabled {
if chatConfig.HookAllowInsecure.Value() && chatConfig.HookURL.Value().Scheme == "http" {
options.Logger.Warn(ctx, "chat hooks use a plain HTTP URL; hook traffic is unencrypted and hook responses controlling agent execution can be forged on the network",
slog.F("hook_url", mcpclient.RedactURL(chatConfig.HookURL.String())),
)
}
hookDispatcher = dispatch.New(
options.Logger,
nil,
chatConfig.HookURL.String(),
chatConfig.HookAllowInsecure.Value(),
chatConfig.HookSecret.Value(),
chatConfig.HookTimeout.Value(),
api.DeploymentID,
coderd/x/agenthooks/dispatch/dispatcher.go
+10 -4
View File
@@ -107,9 +107,11 @@ type Dispatcher struct {
}
// validateHookURL requires HTTPS because hook traffic carries sensitive data
// and authorization tokens, and responses can control execution. Plain HTTP
// is allowed only for loopback development consumers.
func validateHookURL(raw string) error {
// and authorization tokens, and responses can control execution. Loopback HTTP
// is allowed by default; allowInsecure permits HTTP for any host.
//
//nolint:revive // allowInsecure is operator configuration, not caller control coupling.
func validateHookURL(raw string, allowInsecure bool) error {
if raw == "" {
return nil
}
@@ -137,6 +139,9 @@ func validateHookURL(raw string) error {
if host == "" {
return xerrors.New("chat hook URL must include a host")
}
if allowInsecure {
return nil
}
if host == "localhost" {
return nil
}
@@ -155,6 +160,7 @@ func New(
logger slog.Logger,
client *http.Client,
hookURL string,
allowInsecureURL bool,
secret string,
timeout time.Duration,
deploymentID string,
@@ -174,7 +180,7 @@ func New(
logger: logger.Named("chat_hook_dispatcher"),
client: client,
hookURL: hookURL,
hookURLErr: validateHookURL(hookURL),
hookURLErr: validateHookURL(hookURL, allowInsecureURL),
secret: []byte(secret),
timeout: timeout,
deploymentID: deploymentID,
coderd/x/agenthooks/dispatch/dispatcher_internal_test.go
+23 -15
View File
@@ -78,18 +78,25 @@ func TestDispatcherRejectsCleartextURL(t *testing.T) {
_, _, err := dispatcher.Dispatch(testutil.Context(t, testutil.WaitShort), event)
require.ErrorContains(t, err, "must use HTTPS")
require.NoError(t, validateHookURL(""))
require.NoError(t, validateHookURL("https://hooks.example.com/coder"))
require.NoError(t, validateHookURL("http://localhost:8080/hooks"))
require.NoError(t, validateHookURL("http://127.0.0.1:8080/hooks"))
require.NoError(t, validateHookURL("http://[::1]:8080/hooks"))
require.Error(t, validateHookURL("http://10.0.0.5/hooks"))
require.Error(t, validateHookURL("ftp://hooks.example.com/coder"))
require.ErrorContains(t, validateHookURL("https:///coder"), "must include a host")
require.ErrorContains(t, validateHookURL("https:hooks.example.com"), "must include a host")
require.ErrorContains(t, validateHookURL("http:///hooks"), "must include a host")
require.ErrorContains(t, validateHookURL("https://hooks.example.com/coder#frag"), "must not contain a fragment")
require.ErrorContains(t, validateHookURL("https://user:pass@hooks.example.com/coder"), "must not contain userinfo")
require.NoError(t, validateHookURL("", false))
require.NoError(t, validateHookURL("https://hooks.example.com/coder", false))
require.NoError(t, validateHookURL("http://localhost:8080/hooks", false))
require.NoError(t, validateHookURL("http://127.0.0.1:8080/hooks", false))
require.NoError(t, validateHookURL("http://[::1]:8080/hooks", false))
require.Error(t, validateHookURL("http://10.0.0.5/hooks", false))
require.Error(t, validateHookURL("ftp://hooks.example.com/coder", false))
require.ErrorContains(t, validateHookURL("https:///coder", false), "must include a host")
require.ErrorContains(t, validateHookURL("https:hooks.example.com", false), "must include a host")
require.ErrorContains(t, validateHookURL("http:///hooks", false), "must include a host")
require.ErrorContains(t, validateHookURL("https://hooks.example.com/coder#frag", false), "must not contain a fragment")
require.ErrorContains(t, validateHookURL("https://user:pass@hooks.example.com/coder", false), "must not contain userinfo")
require.NoError(t, validateHookURL("http://10.0.0.5/hooks", true))
require.NoError(t, validateHookURL("http://hooks.example.com/coder", true))
require.Error(t, validateHookURL("ftp://hooks.example.com/coder", true))
require.ErrorContains(t, validateHookURL("http:///hooks", true), "must include a host")
require.ErrorContains(t, validateHookURL("http://hooks.example.com/coder#frag", true), "must not contain a fragment")
require.ErrorContains(t, validateHookURL("http://user:pass@hooks.example.com/coder", true), "must not contain userinfo")
}
func TestDispatcherDeny(t *testing.T) {
@@ -566,7 +573,7 @@ func TestDispatcherRejectedResponseIsNotObserved(t *testing.T) {
registry := prometheus.NewRegistry()
dispatcher := New(
testutil.Logger(t), server.Client(), server.URL, testSecret, time.Second,
testutil.Logger(t), server.Client(), server.URL, false, testSecret, time.Second,
testDeploymentID, testVersion, registry,
)
_, _, err := dispatcher.Dispatch(testutil.Context(t, testutil.WaitLong), event)
@@ -655,6 +662,7 @@ func newTestDispatcher(
testutil.Logger(t),
client,
hookURL,
false,
testSecret,
timeout,
testDeploymentID,
@@ -738,7 +746,7 @@ func TestDispatcherAdmissionReserve(t *testing.T) {
t.Cleanup(server.Close)
dispatcher := New(
testutil.Logger(t), server.Client(), server.URL, testSecret, testutil.WaitShort,
testutil.Logger(t), server.Client(), server.URL, false, testSecret, testutil.WaitShort,
testDeploymentID, testVersion, prometheus.NewRegistry(),
)
event := newTestEvent(t, agenthooks.EventUserPromptSubmit, agenthooks.UserPromptSubmitData{Prompt: "hi"})
@@ -797,7 +805,7 @@ func TestDispatcherAdmissionReserve(t *testing.T) {
t.Cleanup(server.Close)
dispatcher := New(
testutil.Logger(t), server.Client(), server.URL, testSecret, testutil.WaitShort,
testutil.Logger(t), server.Client(), server.URL, false, testSecret, testutil.WaitShort,
testDeploymentID, testVersion, prometheus.NewRegistry(),
)
fill(t, dispatcher.admission, maxAdmissionDispatches)
coderd/x/chatd/chathooks/hooks_internal_test.go
+2
View File
@@ -51,6 +51,7 @@ func TestSessionStartDispatchSources(t *testing.T) {
slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
consumer.Client(),
consumer.URL,
false,
secret,
time.Second,
"test-deployment",
@@ -102,6 +103,7 @@ func newTestTrigger(t *testing.T, handler http.Handler) *Trigger {
slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
consumer.Client(),
consumer.URL,
false,
"test-hook-secret-32-bytes-minimum!!",
time.Second,
"test-deployment",
coderd/x/chatd/hooks_internal_test.go
+1
View File
@@ -59,6 +59,7 @@ func TestSessionStartDispatchFailureFinishesGeneration(t *testing.T) {
slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
consumer.Client(),
consumer.URL,
false,
"test-hook-secret-32-bytes-minimum!!",
time.Second,
"test-deployment",
coderd/x/chatd/hooks_test.go
+1
View File
@@ -121,6 +121,7 @@ func newHookDispatcher(t *testing.T, _ database.Store, consumer *httptest.Server
slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
consumer.Client(),
consumer.URL,
false,
"test-hook-secret-32-bytes-minimum!!",
time.Second,
"test-deployment",
coderd/x/chatd/subagent_internal_test.go
+1
View File
@@ -298,6 +298,7 @@ func TestCreateChildSubagentChatDispatchesUserPromptSubmit(t *testing.T) {
slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
consumer.Client(),
consumer.URL,
false,
"test-hook-secret-32-bytes-minimum!!",
time.Second,
"test-deployment",