clawbot
3925fce24a
All checks were successful
check / check (push) Successful in 2m54s
internal/handlers/source_management.go read the target destination
with r.FormValue, which falls back to the URL query string when the
field is absent from the body. So
POST /source/{id}/targets?url=https://hooks.slack.com/services/T/B/S
created a working target from a value carried on the request line,
where logs, proxies, Referer headers and error trackers record it.
That is the remaining ingress path of the credential-exposure class
the render, delivery-error and log-line paths were each closed for.
Every form read in these handlers is now r.PostFormValue, so no
query-string value can populate stored configuration or be taken as a
credential. The one deliberate query read, `page` on the authenticated
pagination links, is untouched: it uses r.URL.Query().Get already.
The access log no longer carries the query on any branch, so the log
half of the report is already mitigated; the Sentry half is not. The
SDK attaches the request to every captured event and copies
r.URL.RawQuery into Request.QueryString independently of the access
log, so a BeforeSend hook clears that field before an event leaves the
process. Scheme, host, path and method stay, which is what names the
failing route.
Second barrier, for the JSON path that does not exist yet: the fields
that hold a credential are tagged json:"-" so the first handler to
marshal a model cannot serialise one. Target.Config holds the
incoming-webhook URL, APIKey.Key is a bearer token, and Setting.Value
holds the session encryption key. delivery.TargetView remains the
masking barrier for the HTML path, which is unaffected.
108 lines
2.6 KiB
Go
108 lines
2.6 KiB
Go
package database_test
|
|
|
|
import (
|
|
"encoding/json"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
)
|
|
|
|
// keptField is a non-secret value planted alongside each secret, so
|
|
// the assertions below cannot pass by the model marshalling to nothing.
|
|
const keptField = "keepme"
|
|
|
|
// marshalModel encodes a model the way a future JSON handler would.
|
|
func marshalModel(t *testing.T, v any) string {
|
|
t.Helper()
|
|
|
|
encoded, err := json.Marshal(v)
|
|
require.NoError(t, err)
|
|
|
|
return string(encoded)
|
|
}
|
|
|
|
// TestModelsDoNotMarshalTheirSecrets pins the barrier for the JSON
|
|
// path. The /api/v1 route group exists and is empty; delivery's
|
|
// TargetView masks the credential for the HTML path only, so without
|
|
// these tags the first handler that marshals a model serialises the
|
|
// secret with it. Each field below is a live credential:
|
|
//
|
|
// - Target.Config holds an incoming-webhook URL whose path segments
|
|
// are the bearer token.
|
|
// - APIKey.Key is a bearer token outright.
|
|
// - Setting.Value holds the session encryption key.
|
|
// - User.Password holds the Argon2 hash, and was already tagged.
|
|
func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
const marker = "QQMODELMARKERQQ"
|
|
|
|
cases := []struct {
|
|
name string
|
|
model any
|
|
}{
|
|
{
|
|
name: "target config",
|
|
model: database.Target{
|
|
Name: keptField,
|
|
Type: database.TargetTypeSlack,
|
|
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
|
},
|
|
},
|
|
{
|
|
name: "api key",
|
|
model: database.APIKey{
|
|
Description: keptField,
|
|
Key: marker,
|
|
},
|
|
},
|
|
{
|
|
name: "setting value",
|
|
model: database.Setting{
|
|
Key: keptField,
|
|
Value: marker,
|
|
},
|
|
},
|
|
{
|
|
name: "user password hash",
|
|
model: database.User{
|
|
Username: keptField,
|
|
Password: marker,
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
encoded := marshalModel(t, tc.model)
|
|
|
|
assert.NotContains(t, encoded, marker)
|
|
assert.Contains(t, encoded, keptField)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestWebhookMarshalsNoTargetConfig covers the nested case: a webhook
|
|
// marshalled with its targets preloaded must not carry the credential
|
|
// through the association either.
|
|
func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
const marker = "QQNESTEDMARKERQQ"
|
|
|
|
encoded := marshalModel(t, database.Webhook{
|
|
Name: keptField,
|
|
Targets: []database.Target{{
|
|
Name: "slack",
|
|
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
|
}},
|
|
})
|
|
|
|
assert.NotContains(t, encoded, marker)
|
|
assert.Contains(t, encoded, keptField)
|
|
}
|