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

chore: forbid direct response body JSON decode in codersdk (#27859)

Browse Source 
Add a ruleguard rule forbidding direct
`json.NewDecoder(res.Body).Decode(...)` on `*http.Response` in codersdk
packages, so new typed endpoints use `codersdk.ReadBodyAsJSON` and keep
returning structured errors for non-JSON bodies. The rule matches both
the chained call form and decoders assigned to a variable first.

Intentional raw-body paths carry documented `//nolint:gocritic`
exceptions: the 16 agent-direct HTTP decodes in
`workspacesdk/agentconn.go` route through a single `decodeAgentJSON`
helper (agent-direct over tailnet, so `ReadBodyAsJSON`'s reverse
proxy/SSO error guidance does not apply), and the Azure IMDS
attested-document decode in `agentsdk/azure.go` keeps an inline
exception.

The two `UseNumber` decoders in `licenses.go` are migrated to a new
`codersdk.ReadBodyAsJSONUseNumber`, so `coder licenses add/list` also
return structured errors for non-JSON bodies instead of `invalid
character '<' looking for beginning of value`.

Note for local verification: golangci-lint caches results, so run
`golangci-lint cache clean` after modifying `scripts/rules.go` or the
rule may silently not fire.

Final PR of the stack on #27804, #27857, and #27858. Refs #27044.


Stack plan

Inventory (full-tree audit): 280 migratable call sites across 47 files;
17 excluded (16 agent-direct HTTP sites in `workspacesdk/agentconn.go`,
1 Azure IMDS decode in `agentsdk/azure.go`).

1. **#27857** `refactor(codersdk): use ReadBodyAsJSON in typed
endpoints`: mechanical migration of all sites except `chats.go` (224
sites, 46 files).
2. **#27858** `refactor(codersdk): use shared error helpers in chat
endpoints`: migrate the 56 `chats.go` sites and consolidate the
duplicated `readRawBodyAsError`/`newResponseError` helpers onto the
shared `client.go` error path, with regression tests for the 409
usage-limit flow.
3. **#27859** `chore: forbid direct response body JSON decode in
codersdk`: ruleguard rule with documented exceptions for the intentional
raw-body paths, plus `ReadBodyAsJSONUseNumber` for the `licenses.go`
decoders.



Reviewed and updated by Coder Agents on behalf of @dylanhuff-at-coder.
This commit is contained in:
dylanhuff-at-coder
2026-08-06 08:03:47 -07:00
committed by GitHub
parent 4e2620d64f
commit 9b27d12929
6 changed files with 99 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codersdk/agentsdk/azure.go
+1 -1
View File
@@ -48,7 +48,7 @@ func (a *AzureSessionTokenExchanger) exchange(ctx context.Context) (Authenticate
defer res.Body.Close()
var token AzureInstanceIdentityToken
err = json.NewDecoder(res.Body).Decode(&token)
err = json.NewDecoder(res.Body).Decode(&token) //nolint:gocritic // Azure IMDS attested document response, not the Coder API.
if err != nil {
return AuthenticateResponse{}, err
}
codersdk/client.go
+22 -1
View File
@@ -519,6 +519,23 @@ const htmlResponseHelper = "Ensure the Coder URL is correct and that any reverse
// API endpoints never serve HTML. The caller remains responsible for
// closing the response body.
func ReadBodyAsJSON(res *http.Response, v any) error {
return decodeBodyAsJSON(res, v, nil)
}
// ReadBodyAsJSONUseNumber behaves like ReadBodyAsJSON but decodes JSON
// numbers into json.Number instead of float64, preserving integer
// precision for callers that re-serialize or type-assert numeric
// claims, such as license JWT claims.
func ReadBodyAsJSONUseNumber(res *http.Response, v any) error {
return decodeBodyAsJSON(res, v, func(dec *json.Decoder) {
dec.UseNumber()
})
}
// decodeBodyAsJSON decodes the response body as JSON into v. When
// configure is non-nil it is called with the decoder before decoding,
// allowing callers to set options such as UseNumber.
func decodeBodyAsJSON(res *http.Response, v any, configure func(*json.Decoder)) error {
if res == nil || res.Body == nil {
return xerrors.New("no response body to decode")
}
@@ -530,7 +547,11 @@ func ReadBodyAsJSON(res *http.Response, v any) error {
body := &responseBodyReader{Reader: res.Body}
prefix := &bodyPrefixWriter{}
err := json.NewDecoder(io.TeeReader(body, prefix)).Decode(v)
dec := json.NewDecoder(io.TeeReader(body, prefix))
if configure != nil {
configure(dec)
}
err := dec.Decode(v)
switch {
case err == nil:
return nil
codersdk/client_internal_test.go
+18
View File
@@ -640,6 +640,24 @@ func Test_ReadBodyAsJSON(t *testing.T) {
require.ErrorContains(t, err, "read response body")
require.NotContains(t, err.Error(), "invalid API response")
})
//nolint:bodyclose // The response is constructed, not from a client.
t.Run("UseNumber", func(t *testing.T) {
t.Parallel()
var v map[string]any
res := newResponse(http.StatusOK, jsonCT, `{"exp":1750000000}`)
require.NoError(t, ReadBodyAsJSONUseNumber(res, &v))
num, ok := v["exp"].(json.Number)
require.True(t, ok, "expected json.Number, got %T", v["exp"])
require.Equal(t, "1750000000", num.String())
// ReadBodyAsJSON decodes the same body's numbers as float64.
res = newResponse(http.StatusOK, jsonCT, `{"exp":1750000000}`)
require.NoError(t, ReadBodyAsJSON(res, &v))
_, ok = v["exp"].(float64)
require.True(t, ok, "expected float64, got %T", v["exp"])
})
}
func assertSDKError(t *testing.T, err error) *Error {
codersdk/licenses.go
+2 -6
View File
@@ -106,9 +106,7 @@ func (c *Client) AddLicense(ctx context.Context, r AddLicenseRequest) (License,
return License{}, ReadBodyAsError(res)
}
var l License
d := json.NewDecoder(res.Body)
d.UseNumber()
return l, d.Decode(&l)
return l, ReadBodyAsJSONUseNumber(res, &l)
}
func (c *Client) Licenses(ctx context.Context) ([]License, error) {
@@ -121,9 +119,7 @@ func (c *Client) Licenses(ctx context.Context) ([]License, error) {
return nil, ReadBodyAsError(res)
}
var licenses []License
d := json.NewDecoder(res.Body)
d.UseNumber()
return licenses, d.Decode(&licenses)
return licenses, ReadBodyAsJSONUseNumber(res, &licenses)
}
func (c *Client) DeleteLicense(ctx context.Context, id int32) error {
codersdk/workspacesdk/agentconn.go
+27 -16
View File
@@ -404,7 +404,7 @@ func (c *agentConn) ListeningPorts(ctx context.Context) (codersdk.WorkspaceAgent
}
var resp codersdk.WorkspaceAgentListeningPortsResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// Netcheck returns a network check report from the workspace agent.
@@ -421,7 +421,7 @@ func (c *agentConn) Netcheck(ctx context.Context) (healthsdk.AgentNetcheckReport
}
var resp healthsdk.AgentNetcheckReport
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// DebugMagicsock makes a request to the workspace agent's magicsock debug endpoint.
@@ -607,7 +607,7 @@ func (c *agentConn) ListContainers(ctx context.Context) (codersdk.WorkspaceAgent
return codersdk.WorkspaceAgentListContainersResponse{}, codersdk.ReadBodyAsError(res)
}
var resp codersdk.WorkspaceAgentListContainersResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
func (c *agentConn) WatchContainers(ctx context.Context, logger slog.Logger) (<-chan codersdk.WorkspaceAgentListContainersResponse, io.Closer, error) {
@@ -820,7 +820,7 @@ func (c *agentConn) ExecuteDesktopAction(ctx context.Context, action DesktopActi
}
var result DesktopActionResponse
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
if err := decodeAgentJSON(resp, &result); err != nil {
return DesktopActionResponse{}, xerrors.Errorf("decode action response: %w", err)
}
return result, nil
@@ -898,7 +898,7 @@ func (c *agentConn) RecreateDevcontainer(ctx context.Context, devcontainerID str
return codersdk.Response{}, codersdk.ReadBodyAsError(res)
}
var m codersdk.Response
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
if err := decodeAgentJSON(res, &m); err != nil {
return codersdk.Response{}, xerrors.Errorf("decode response body: %w", err)
}
return m, nil
@@ -1017,7 +1017,7 @@ func (c *agentConn) LS(ctx context.Context, path string, req LSRequest) (LSRespo
}
var m LSResponse
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
if err := decodeAgentJSON(res, &m); err != nil {
return LSResponse{}, xerrors.Errorf("decode response body: %w", err)
}
return m, nil
@@ -1046,7 +1046,7 @@ func (c *agentConn) ResolvePath(ctx context.Context, path string) (string, error
}
var m ResolvePathResponse
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
if err := decodeAgentJSON(res, &m); err != nil {
return "", xerrors.Errorf("decode response body: %w", err)
}
return m.ResolvedPath, nil
@@ -1076,7 +1076,7 @@ func (c *agentConn) ReadFileLines(ctx context.Context, path string, offset, limi
}
var resp ReadFileLinesResponse
if err := json.NewDecoder(res.Body).Decode(&resp); err != nil {
if err := decodeAgentJSON(res, &resp); err != nil {
return ReadFileLinesResponse{}, xerrors.Errorf("decode response: %w", err)
}
return resp, nil
@@ -1127,7 +1127,7 @@ func (c *agentConn) WriteFile(ctx context.Context, path string, reader io.Reader
}
var m codersdk.Response
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
if err := decodeAgentJSON(res, &m); err != nil {
return xerrors.Errorf("decode response body: %w", err)
}
return nil
@@ -1277,7 +1277,7 @@ func (c *agentConn) StartProcess(ctx context.Context, req StartProcessRequest) (
return StartProcessResponse{}, codersdk.ReadBodyAsError(res)
}
var resp StartProcessResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// ListProcesses returns information about tracked processes on the agent.
@@ -1293,7 +1293,7 @@ func (c *agentConn) ListProcesses(ctx context.Context) (ListProcessesResponse, e
return ListProcessesResponse{}, codersdk.ReadBodyAsError(res)
}
var resp ListProcessesResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// ContextConfig returns the resolved context configuration from
@@ -1310,7 +1310,7 @@ func (c *agentConn) ContextConfig(ctx context.Context) (ContextConfigResponse, e
return ContextConfigResponse{}, codersdk.ReadBodyAsError(res)
}
var resp ContextConfigResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// CallMCPTool proxies a tool call to an MCP server running in
@@ -1327,7 +1327,7 @@ func (c *agentConn) CallMCPTool(ctx context.Context, req CallMCPToolRequest) (Ca
return CallMCPToolResponse{}, codersdk.ReadBodyAsError(res)
}
var resp CallMCPToolResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// ProcessOutput returns the output of a tracked process on the agent.
@@ -1347,7 +1347,7 @@ func (c *agentConn) ProcessOutput(ctx context.Context, id string, opts *ProcessO
return ProcessOutputResponse{}, codersdk.ReadBodyAsError(res)
}
var resp ProcessOutputResponse
return resp, json.NewDecoder(res.Body).Decode(&resp)
return resp, decodeAgentJSON(res, &resp)
}
// SignalProcess sends a signal to a tracked process on the agent.
@@ -1363,7 +1363,7 @@ func (c *agentConn) SignalProcess(ctx context.Context, id string, signal string)
return codersdk.ReadBodyAsError(res)
}
var m codersdk.Response
if err := json.NewDecoder(res.Body).Decode(&m); err != nil {
if err := decodeAgentJSON(res, &m); err != nil {
return xerrors.Errorf("decode response body: %w", err)
}
return nil
@@ -1386,7 +1386,7 @@ func (c *agentConn) EditFiles(ctx context.Context, edits FileEditRequest) (FileE
}
var resp FileEditResponse
if err := json.NewDecoder(res.Body).Decode(&resp); err != nil {
if err := decodeAgentJSON(res, &resp); err != nil {
return FileEditResponse{}, xerrors.Errorf("decode response body: %w", err)
}
return resp, nil
@@ -1445,6 +1445,17 @@ func (c *agentConn) apiRequest(ctx context.Context, method, path string, body in
return c.apiClient(ctx).Do(req)
}
// decodeAgentJSON decodes an agent-direct HTTP response body. Agent
// endpoints are served by the workspace agent over tailnet, not the
// Coder control plane, so no reverse proxy or SSO portal can intercept
// the response and codersdk.ReadBodyAsJSON's proxy/SSO-oriented error
// guidance would be misleading here.
//
//nolint:gocritic // See doc comment.
func decodeAgentJSON(res *http.Response, v any) error {
return json.NewDecoder(res.Body).Decode(v)
}
// apiClient returns an HTTP client that can be used to make
// requests to the workspace agent's HTTP API server. The client is
// scoped to a single request: its transport cancels in-flight dials