Compare commits
2 Commits
72868c0f02
...
next
| Author | SHA1 | Date | |
|---|---|---|---|
| aab448b076 | |||
| 7c43e095a6 |
@@ -92,7 +92,12 @@ func ValidateTargetURL(
|
|||||||
) error {
|
) error {
|
||||||
parsed, err := url.Parse(targetURL)
|
parsed, err := url.Parse(targetURL)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("invalid URL: %w", err)
|
// url.Parse embeds the whole URL in its error, and
|
||||||
|
// this one is logged and shown; mask it. Every other
|
||||||
|
// branch below reports only the hostname.
|
||||||
|
return fmt.Errorf(
|
||||||
|
"invalid URL: %w", maskURLError(err),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = validateScheme(parsed.Scheme)
|
err = validateScheme(parsed.Scheme)
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ package delivery
|
|||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/url"
|
|
||||||
"strconv"
|
"strconv"
|
||||||
|
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
@@ -17,9 +16,6 @@ import (
|
|||||||
// browser history, screenshots and screen shares.
|
// browser history, screenshots and screen shares.
|
||||||
const configUnavailable = "(unavailable)"
|
const configUnavailable = "(unavailable)"
|
||||||
|
|
||||||
// urlPathElision stands in for a URL's elided path.
|
|
||||||
const urlPathElision = "/..."
|
|
||||||
|
|
||||||
// ConfigField is one labelled, display-safe value derived
|
// ConfigField is one labelled, display-safe value derived
|
||||||
// from a target's stored configuration.
|
// from a target's stored configuration.
|
||||||
type ConfigField struct {
|
type ConfigField struct {
|
||||||
@@ -202,23 +198,5 @@ func databaseConfigFields(configJSON string) []ConfigField {
|
|||||||
// parse into a scheme and host yields the neutral
|
// parse into a scheme and host yields the neutral
|
||||||
// placeholder, never the raw string.
|
// placeholder, never the raw string.
|
||||||
func (c *SlackTargetConfig) MaskedWebhookURL() string {
|
func (c *SlackTargetConfig) MaskedWebhookURL() string {
|
||||||
return maskURL(c.WebhookURL)
|
return MaskURL(c.WebhookURL)
|
||||||
}
|
|
||||||
|
|
||||||
// maskURL renders a URL as scheme plus host with everything
|
|
||||||
// that can carry a secret removed.
|
|
||||||
func maskURL(raw string) string {
|
|
||||||
parsed, err := url.Parse(raw)
|
|
||||||
if err != nil || parsed.Scheme == "" ||
|
|
||||||
parsed.Host == "" {
|
|
||||||
return configUnavailable
|
|
||||||
}
|
|
||||||
|
|
||||||
masked := parsed.Scheme + "://" + parsed.Host
|
|
||||||
|
|
||||||
if parsed.Path != "" && parsed.Path != "/" {
|
|
||||||
masked += urlPathElision
|
|
||||||
}
|
|
||||||
|
|
||||||
return masked
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -363,7 +363,8 @@ func (t *httpTarget) doHTTPRequest(
|
|||||||
)
|
)
|
||||||
if reqErr != nil {
|
if reqErr != nil {
|
||||||
return 0, "", 0, fmt.Errorf(
|
return 0, "", 0, fmt.Errorf(
|
||||||
"creating request: %w", reqErr,
|
"creating request: %w",
|
||||||
|
maskURLError(reqErr),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -492,8 +493,19 @@ func applyRequestHeaders(
|
|||||||
// executeHTTPRequest sends an HTTP request using the provided
|
// executeHTTPRequest sends an HTTP request using the provided
|
||||||
// client. URLs are validated by the config parsers and the
|
// client. URLs are validated by the config parsers and the
|
||||||
// SSRF-safe transport before reaching here.
|
// SSRF-safe transport before reaching here.
|
||||||
|
//
|
||||||
|
// Transport failures are masked here, at the single point
|
||||||
|
// where every target's request errors are born, because the
|
||||||
|
// caller stores them in DeliveryResult.Error: an unmasked
|
||||||
|
// *url.Error would write the target URL — the credential for
|
||||||
|
// a Slack incoming webhook — into the per-webhook database.
|
||||||
func executeHTTPRequest(
|
func executeHTTPRequest(
|
||||||
client *http.Client, req *http.Request,
|
client *http.Client, req *http.Request,
|
||||||
) (*http.Response, error) {
|
) (*http.Response, error) {
|
||||||
return client.Do(req) //#nosec G704 -- validated URL, SSRF-safe transport
|
resp, err := client.Do(req) //#nosec G704 -- validated URL, SSRF-safe transport
|
||||||
|
if err != nil {
|
||||||
|
return nil, maskURLError(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return resp, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ func (t *slackTarget) attempt(
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return attemptResult{
|
return attemptResult{
|
||||||
success: false,
|
success: false,
|
||||||
errMsg: err.Error(),
|
errMsg: maskURLError(err).Error(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
61
internal/delivery/url_mask.go
Normal file
61
internal/delivery/url_mask.go
Normal file
@@ -0,0 +1,61 @@
|
|||||||
|
package delivery
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"net/url"
|
||||||
|
)
|
||||||
|
|
||||||
|
// urlPathElision stands in for a URL's elided path.
|
||||||
|
const urlPathElision = "/..."
|
||||||
|
|
||||||
|
// MaskURL renders a URL as scheme plus host with everything
|
||||||
|
// that can carry a secret removed. A delivery target URL is
|
||||||
|
// itself a credential — a Slack incoming webhook URL is a
|
||||||
|
// bearer token — so the path, query and userinfo are never
|
||||||
|
// reproduced, in a page, a log line or a stored error. A URL
|
||||||
|
// that does not parse into a scheme and host yields the
|
||||||
|
// neutral placeholder, never the raw string.
|
||||||
|
func MaskURL(raw string) string {
|
||||||
|
parsed, err := url.Parse(raw)
|
||||||
|
if err != nil || parsed.Scheme == "" ||
|
||||||
|
parsed.Host == "" {
|
||||||
|
return configUnavailable
|
||||||
|
}
|
||||||
|
|
||||||
|
masked := parsed.Scheme + "://" + parsed.Host
|
||||||
|
|
||||||
|
if parsed.Path != "" && parsed.Path != "/" {
|
||||||
|
masked += urlPathElision
|
||||||
|
}
|
||||||
|
|
||||||
|
return masked
|
||||||
|
}
|
||||||
|
|
||||||
|
// maskURLError strips the credential from an error raised
|
||||||
|
// against a request URL. The net/http and net/url packages
|
||||||
|
// embed the full request URL in every *url.Error they return,
|
||||||
|
// so an unmodified transport error persisted into
|
||||||
|
// DeliveryResult.Error writes the credential to disk.
|
||||||
|
//
|
||||||
|
// The masked error keeps the operation and the wrapped cause,
|
||||||
|
// so a DNS failure still reads differently from a refused
|
||||||
|
// connection, a TLS handshake failure or a timeout, and Is,
|
||||||
|
// As, Timeout and Temporary keep working on it. Only the
|
||||||
|
// path, query and userinfo of the URL are dropped. Errors
|
||||||
|
// that carry no URL are returned unchanged.
|
||||||
|
//
|
||||||
|
// Call it where the error is raised, before any wrapping: it
|
||||||
|
// replaces the *url.Error itself, so any context wrapped
|
||||||
|
// around it first would be discarded.
|
||||||
|
func maskURLError(err error) error {
|
||||||
|
var urlErr *url.Error
|
||||||
|
if !errors.As(err, &urlErr) {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return &url.Error{
|
||||||
|
Op: urlErr.Op,
|
||||||
|
URL: MaskURL(urlErr.URL),
|
||||||
|
Err: urlErr.Err,
|
||||||
|
}
|
||||||
|
}
|
||||||
196
internal/delivery/url_mask_test.go
Normal file
196
internal/delivery/url_mask_test.go
Normal file
@@ -0,0 +1,196 @@
|
|||||||
|
package delivery_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"gorm.io/gorm"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/delivery"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The path of a Slack incoming webhook URL is the credential:
|
||||||
|
// whoever holds these segments can post to the channel
|
||||||
|
// forever. None of them may reach a stored delivery error,
|
||||||
|
// which lives on disk in the per-webhook database and is
|
||||||
|
// serialized by the JSON tag on DeliveryResult.Error.
|
||||||
|
const (
|
||||||
|
maskSecretPath = "/services/T00000000/B00000000/" +
|
||||||
|
"XXXXXXXXXXXXXXXXXXXXXXXX"
|
||||||
|
)
|
||||||
|
|
||||||
|
// assertNoCredential fails if the whole path or any single
|
||||||
|
// segment of it survived into the message, so a partial leak
|
||||||
|
// fails the test too.
|
||||||
|
func assertNoCredential(t *testing.T, msg string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
segments := []string{
|
||||||
|
maskSecretPath,
|
||||||
|
"services",
|
||||||
|
"T00000000",
|
||||||
|
"B00000000",
|
||||||
|
"XXXXXXXXXXXXXXXXXXXXXXXX",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, segment := range segments {
|
||||||
|
assert.NotContains(t, msg, segment)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// storedDeliveryError returns the error string persisted for a
|
||||||
|
// delivery, which is what an operator and any future API read.
|
||||||
|
func storedDeliveryError(
|
||||||
|
t *testing.T, db *gorm.DB, deliveryID string,
|
||||||
|
) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var result database.DeliveryResult
|
||||||
|
|
||||||
|
require.NoError(t, db.Where(
|
||||||
|
"delivery_id = ?", deliveryID,
|
||||||
|
).First(&result).Error)
|
||||||
|
|
||||||
|
return result.Error
|
||||||
|
}
|
||||||
|
|
||||||
|
// deliverSlackTo runs a Slack delivery against webhookURL and
|
||||||
|
// returns the error string it persisted.
|
||||||
|
func deliverSlackTo(
|
||||||
|
t *testing.T, webhookURL string,
|
||||||
|
) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
db := testWebhookDB(t)
|
||||||
|
e := testEngine(t, 1)
|
||||||
|
targetID := uuid.New().String()
|
||||||
|
|
||||||
|
slackCfg, err := json.Marshal(
|
||||||
|
delivery.SlackTargetConfig{
|
||||||
|
WebhookURL: webhookURL,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
event := seedEvent(t, db, `{"test":true}`)
|
||||||
|
|
||||||
|
dlv := seedDelivery(
|
||||||
|
t, db, event.ID, targetID,
|
||||||
|
database.DeliveryStatusPending,
|
||||||
|
)
|
||||||
|
|
||||||
|
d := buildSlackDelivery(
|
||||||
|
dlv, event, targetID,
|
||||||
|
"test-slack-mask", string(slackCfg),
|
||||||
|
)
|
||||||
|
|
||||||
|
e.ExportDeliverSlack(context.TODO(), db, d)
|
||||||
|
|
||||||
|
assertDeliveryStatus(t, db, dlv.ID,
|
||||||
|
database.DeliveryStatusFailed,
|
||||||
|
)
|
||||||
|
|
||||||
|
return storedDeliveryError(t, db, dlv.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeliverSlack_TransportErrorMasksWebhookURL is the
|
||||||
|
// load-bearing regression test: a transport failure must not
|
||||||
|
// persist the webhook URL's credential into the database, and
|
||||||
|
// must still say what went wrong and where.
|
||||||
|
func TestDeliverSlack_TransportErrorMasksWebhookURL(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A server closed before use gives a deterministic
|
||||||
|
// transport failure against a known host.
|
||||||
|
ts := httptest.NewServer(http.NewServeMux())
|
||||||
|
host := ts.URL
|
||||||
|
|
||||||
|
ts.Close()
|
||||||
|
|
||||||
|
errMsg := deliverSlackTo(t, host+maskSecretPath)
|
||||||
|
|
||||||
|
require.NotEmpty(t, errMsg)
|
||||||
|
assertNoCredential(t, errMsg)
|
||||||
|
|
||||||
|
// The diagnostic value survives: the operation, the host
|
||||||
|
// and the transport failure are all still reported, and
|
||||||
|
// only the path is elided.
|
||||||
|
assert.Contains(t, errMsg, "sending request")
|
||||||
|
assert.Contains(t, errMsg, "Post")
|
||||||
|
assert.Contains(t, errMsg, host+"/...")
|
||||||
|
assert.Contains(t, errMsg, "connection refused")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeliverSlack_UnparsableURLMasksWebhookURL covers the
|
||||||
|
// other error path out of a Slack attempt: url.Parse also
|
||||||
|
// embeds the whole URL in the error it returns.
|
||||||
|
func TestDeliverSlack_UnparsableURLMasksWebhookURL(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
errMsg := deliverSlackTo(
|
||||||
|
t,
|
||||||
|
"https://hooks.slack.com"+maskSecretPath+"\n",
|
||||||
|
)
|
||||||
|
|
||||||
|
require.NotEmpty(t, errMsg)
|
||||||
|
assertNoCredential(t, errMsg)
|
||||||
|
assert.Contains(t, errMsg, "invalid control character")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDoHTTPRequest_TransportErrorMasksURL proves the HTTP
|
||||||
|
// target's transport errors are masked too; its destination
|
||||||
|
// URL can carry a token in a query string.
|
||||||
|
func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ts := httptest.NewServer(http.NewServeMux())
|
||||||
|
host := ts.URL
|
||||||
|
|
||||||
|
ts.Close()
|
||||||
|
|
||||||
|
e := testEngine(t, 1)
|
||||||
|
|
||||||
|
cfg, err := e.ExportParseHTTPConfig(
|
||||||
|
newHTTPTargetConfig(host + maskSecretPath),
|
||||||
|
)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
statusCode, _, _, reqErr := e.ExportDoHTTPRequest(
|
||||||
|
context.TODO(), cfg,
|
||||||
|
&database.Event{Body: `{"test":true}`},
|
||||||
|
)
|
||||||
|
require.Error(t, reqErr)
|
||||||
|
assert.Zero(t, statusCode)
|
||||||
|
|
||||||
|
assertNoCredential(t, reqErr.Error())
|
||||||
|
assert.Contains(t, reqErr.Error(), host+"/...")
|
||||||
|
assert.Contains(
|
||||||
|
t, reqErr.Error(), "connection refused",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestValidateTargetURL_UnparsableURLIsMasked proves the SSRF
|
||||||
|
// validator's error does not carry the submitted URL, which
|
||||||
|
// the handler both logs and shows.
|
||||||
|
func TestValidateTargetURL_UnparsableURLIsMasked(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
err := delivery.ValidateTargetURL(
|
||||||
|
context.TODO(),
|
||||||
|
"https://hooks.slack.com"+maskSecretPath+"\n",
|
||||||
|
)
|
||||||
|
require.Error(t, err)
|
||||||
|
|
||||||
|
assertNoCredential(t, err.Error())
|
||||||
|
assert.Contains(t, err.Error(), "invalid URL")
|
||||||
|
}
|
||||||
@@ -26,14 +26,14 @@ const (
|
|||||||
)
|
)
|
||||||
|
|
||||||
// seedConfiguredTarget inserts a target with a stored config
|
// seedConfiguredTarget inserts a target with a stored config
|
||||||
// blob.
|
// blob and returns it.
|
||||||
func seedConfiguredTarget(
|
func seedConfiguredTarget(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
db *database.Database,
|
db *database.Database,
|
||||||
webhookID string,
|
webhookID string,
|
||||||
targetType database.TargetType,
|
targetType database.TargetType,
|
||||||
config string,
|
config string,
|
||||||
) {
|
) *database.Target {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
tgt := &database.Target{
|
tgt := &database.Target{
|
||||||
@@ -48,6 +48,8 @@ func seedConfiguredTarget(
|
|||||||
t,
|
t,
|
||||||
db.DB().Omit(clause.Associations).Create(tgt).Error,
|
db.DB().Omit(clause.Associations).Create(tgt).Error,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
return tgt
|
||||||
}
|
}
|
||||||
|
|
||||||
// renderSourceDetailPage runs the real source detail handler
|
// renderSourceDetailPage runs the real source detail handler
|
||||||
|
|||||||
134
internal/handlers/source_logs_test.go
Normal file
134
internal/handlers/source_logs_test.go
Normal file
@@ -0,0 +1,134 @@
|
|||||||
|
package handlers_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"gorm.io/gorm/clause"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/handlers"
|
||||||
|
"sneak.berlin/go/webhooker/internal/session"
|
||||||
|
)
|
||||||
|
|
||||||
|
// seedDeliveredEvent records an event and a delivery for it in
|
||||||
|
// the webhook's own database, so the log page has a delivery
|
||||||
|
// to render against the target.
|
||||||
|
func seedDeliveredEvent(
|
||||||
|
t *testing.T,
|
||||||
|
dbMgr *database.WebhookDBManager,
|
||||||
|
webhookID, targetID string,
|
||||||
|
) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
webhookDB, err := dbMgr.GetDB(webhookID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
event := &database.Event{
|
||||||
|
WebhookID: webhookID,
|
||||||
|
Method: http.MethodPost,
|
||||||
|
Body: `{"test":true}`,
|
||||||
|
ContentType: "application/json",
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, webhookDB.Omit(
|
||||||
|
clause.Associations,
|
||||||
|
).Create(event).Error)
|
||||||
|
|
||||||
|
dlv := &database.Delivery{
|
||||||
|
EventID: event.ID,
|
||||||
|
TargetID: targetID,
|
||||||
|
Status: database.DeliveryStatusDelivered,
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, webhookDB.Omit(
|
||||||
|
clause.Associations,
|
||||||
|
).Create(dlv).Error)
|
||||||
|
}
|
||||||
|
|
||||||
|
// renderSourceLogsPage runs the real event log handler for a
|
||||||
|
// webhook and returns the rendered HTML.
|
||||||
|
func renderSourceLogsPage(
|
||||||
|
t *testing.T,
|
||||||
|
h *handlers.Handlers,
|
||||||
|
sess *session.Session,
|
||||||
|
webhookID string,
|
||||||
|
) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(),
|
||||||
|
http.MethodGet,
|
||||||
|
"/source/"+webhookID+"/logs",
|
||||||
|
nil,
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, c := range authenticatedCookies(
|
||||||
|
t, sess, deleteTestUserID, deleteTestUsername,
|
||||||
|
) {
|
||||||
|
req.AddCookie(c)
|
||||||
|
}
|
||||||
|
|
||||||
|
rctx := chi.NewRouteContext()
|
||||||
|
rctx.URLParams.Add(paramSourceID, webhookID)
|
||||||
|
|
||||||
|
req = req.WithContext(
|
||||||
|
context.WithValue(
|
||||||
|
req.Context(), chi.RouteCtxKey, rctx,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
h.HandleSourceLogs().ServeHTTP(w, req)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
|
||||||
|
return w.Body.String()
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleSourceLogs_MasksSlackWebhookURL proves the event
|
||||||
|
// log page is handed a display-safe projection of each target
|
||||||
|
// rather than the stored row, so the credential cannot be
|
||||||
|
// rendered from its template data.
|
||||||
|
func TestHandleSourceLogs_MasksSlackWebhookURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
tgt := seedConfiguredTarget(
|
||||||
|
t, db, wh.ID,
|
||||||
|
database.TargetTypeSlack,
|
||||||
|
`{"webhookUrl":"`+slackWebhookURL+`"}`,
|
||||||
|
)
|
||||||
|
|
||||||
|
seedDeliveredEvent(t, dbMgr, wh.ID, tgt.ID)
|
||||||
|
|
||||||
|
body := renderSourceLogsPage(t, h, sess, wh.ID)
|
||||||
|
|
||||||
|
assert.NotContains(t, body, slackSecretPath)
|
||||||
|
assert.NotContains(t, body, "T00000000")
|
||||||
|
assert.NotContains(t, body, "B00000000")
|
||||||
|
assert.NotContains(
|
||||||
|
t, body, "XXXXXXXXXXXXXXXXXXXXXXXX",
|
||||||
|
)
|
||||||
|
assert.NotContains(t, body, "webhookUrl")
|
||||||
|
|
||||||
|
// The page still identifies the delivery's target.
|
||||||
|
assert.Contains(t, body, tgt.Name)
|
||||||
|
assert.Contains(t, body, "delivered")
|
||||||
|
}
|
||||||
@@ -96,7 +96,17 @@ func parseRetentionDays(raw string, fallback int) (int, error) {
|
|||||||
type EventWithDeliveries struct {
|
type EventWithDeliveries struct {
|
||||||
database.Event
|
database.Event
|
||||||
|
|
||||||
Deliveries []database.Delivery
|
Deliveries []DeliveryView
|
||||||
|
}
|
||||||
|
|
||||||
|
// DeliveryView is the display-safe projection of a delivery
|
||||||
|
// for the event log page. Its target is a TargetView, so the
|
||||||
|
// stored configuration blob — which holds the target's
|
||||||
|
// credential — has no path to the template.
|
||||||
|
type DeliveryView struct {
|
||||||
|
ID string
|
||||||
|
Status database.DeliveryStatus
|
||||||
|
Target delivery.TargetView
|
||||||
}
|
}
|
||||||
|
|
||||||
// HandleSourceList shows a list of user's webhooks.
|
// HandleSourceList shows a list of user's webhooks.
|
||||||
@@ -764,22 +774,27 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// loadTargetMap loads targets into a map keyed by target ID.
|
// loadTargetMap loads targets into a map of display-safe
|
||||||
|
// views keyed by target ID. The projection happens here so
|
||||||
|
// that no caller can hand a raw target, configuration blob
|
||||||
|
// and all, to a template.
|
||||||
func (h *Handlers) loadTargetMap(
|
func (h *Handlers) loadTargetMap(
|
||||||
webhookID string,
|
webhookID string,
|
||||||
) map[string]database.Target {
|
) map[string]delivery.TargetView {
|
||||||
var targets []database.Target
|
var targets []database.Target
|
||||||
|
|
||||||
h.db.DB().Where(
|
h.db.DB().Where(
|
||||||
"webhook_id = ?", webhookID,
|
"webhook_id = ?", webhookID,
|
||||||
).Find(&targets)
|
).Find(&targets)
|
||||||
|
|
||||||
|
views := delivery.NewTargetViews(targets)
|
||||||
|
|
||||||
targetMap := make(
|
targetMap := make(
|
||||||
map[string]database.Target, len(targets),
|
map[string]delivery.TargetView, len(views),
|
||||||
)
|
)
|
||||||
|
|
||||||
for _, t := range targets {
|
for _, v := range views {
|
||||||
targetMap[t.ID] = t
|
targetMap[v.ID] = v
|
||||||
}
|
}
|
||||||
|
|
||||||
return targetMap
|
return targetMap
|
||||||
@@ -804,7 +819,7 @@ func (h *Handlers) parsePage(r *http.Request) int {
|
|||||||
func (h *Handlers) loadEventsWithDeliveries(
|
func (h *Handlers) loadEventsWithDeliveries(
|
||||||
w http.ResponseWriter,
|
w http.ResponseWriter,
|
||||||
webhook database.Webhook,
|
webhook database.Webhook,
|
||||||
targetMap map[string]database.Target,
|
targetMap map[string]delivery.TargetView,
|
||||||
page int,
|
page int,
|
||||||
) ([]EventWithDeliveries, int64) {
|
) ([]EventWithDeliveries, int64) {
|
||||||
var totalEvents int64
|
var totalEvents int64
|
||||||
@@ -843,22 +858,39 @@ func (h *Handlers) loadEventsWithDeliveries(
|
|||||||
for i := range events {
|
for i := range events {
|
||||||
result[i].Event = events[i]
|
result[i].Event = events[i]
|
||||||
|
|
||||||
|
var deliveries []database.Delivery
|
||||||
|
|
||||||
webhookDB.Where(
|
webhookDB.Where(
|
||||||
"event_id = ?", events[i].ID,
|
"event_id = ?", events[i].ID,
|
||||||
).Find(&result[i].Deliveries)
|
).Find(&deliveries)
|
||||||
|
|
||||||
for j := range result[i].Deliveries {
|
result[i].Deliveries = newDeliveryViews(
|
||||||
tid := result[i].Deliveries[j].TargetID
|
deliveries, targetMap,
|
||||||
|
)
|
||||||
if target, ok := targetMap[tid]; ok {
|
|
||||||
result[i].Deliveries[j].Target = target
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return result, totalEvents
|
return result, totalEvents
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// newDeliveryViews projects deliveries for rendering,
|
||||||
|
// resolving each one's target to its display-safe view.
|
||||||
|
func newDeliveryViews(
|
||||||
|
deliveries []database.Delivery,
|
||||||
|
targetMap map[string]delivery.TargetView,
|
||||||
|
) []DeliveryView {
|
||||||
|
views := make([]DeliveryView, len(deliveries))
|
||||||
|
|
||||||
|
for i := range deliveries {
|
||||||
|
views[i] = DeliveryView{
|
||||||
|
ID: deliveries[i].ID,
|
||||||
|
Status: deliveries[i].Status,
|
||||||
|
Target: targetMap[deliveries[i].TargetID],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return views
|
||||||
|
}
|
||||||
|
|
||||||
// HandleEntrypointCreate handles adding a new entrypoint.
|
// HandleEntrypointCreate handles adding a new entrypoint.
|
||||||
func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -1102,9 +1134,12 @@ func (h *Handlers) buildURLTargetConfig(
|
|||||||
r.Context(), targetURL,
|
r.Context(), targetURL,
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
// The submitted URL can be a credential (a Slack
|
||||||
|
// incoming webhook URL is a bearer token), so the log
|
||||||
|
// records only its scheme and host.
|
||||||
h.log.Warn(
|
h.log.Warn(
|
||||||
"target URL blocked by SSRF protection",
|
"target URL blocked by SSRF protection",
|
||||||
"url", targetURL,
|
"url", delivery.MaskURL(targetURL),
|
||||||
"error", err,
|
"error", err,
|
||||||
)
|
)
|
||||||
http.Error(
|
http.Error(
|
||||||
|
|||||||
@@ -6,9 +6,11 @@ import (
|
|||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"strconv"
|
"strconv"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/delivery"
|
||||||
"sneak.berlin/go/webhooker/internal/handlers"
|
"sneak.berlin/go/webhooker/internal/handlers"
|
||||||
"sneak.berlin/go/webhooker/internal/session"
|
"sneak.berlin/go/webhooker/internal/session"
|
||||||
)
|
)
|
||||||
@@ -21,6 +23,10 @@ const (
|
|||||||
dataKeyError = "Error"
|
dataKeyError = "Error"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// testWebhookID is the identifier given to the webhook under test on
|
||||||
|
// pages that render one.
|
||||||
|
const testWebhookID = "wh-1"
|
||||||
|
|
||||||
// renderPage renders a page template through the real template set as
|
// renderPage renders a page template through the real template set as
|
||||||
// an authenticated user and returns the resulting HTML.
|
// an authenticated user and returns the resulting HTML.
|
||||||
func renderPage(
|
func renderPage(
|
||||||
@@ -62,10 +68,21 @@ func TestNavbarUsesWebhookTerminology(t *testing.T) {
|
|||||||
|
|
||||||
t.Cleanup(app.RequireStop)
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
// One item, so the list body renders too: it calls
|
||||||
|
// WebhookListItem.RetentionLabel, promoted from the embedded
|
||||||
|
// Webhook and therefore a pointer method. An empty list would
|
||||||
|
// skip that call and hide a template error behind the
|
||||||
|
// navigation assertions below.
|
||||||
|
item := handlers.WebhookListItem{}
|
||||||
|
item.Name = "wh"
|
||||||
|
item.ID = testWebhookID
|
||||||
|
item.RetentionDays = 14
|
||||||
|
|
||||||
body := renderPage(t, h, sess, "sources_list.html", map[string]any{
|
body := renderPage(t, h, sess, "sources_list.html", map[string]any{
|
||||||
"Webhooks": []handlers.WebhookListItem{},
|
"Webhooks": []handlers.WebhookListItem{item},
|
||||||
})
|
})
|
||||||
|
|
||||||
|
assert.Contains(t, body, "Retention: 14 days")
|
||||||
assert.Contains(t, body, `class="btn-text">Webhooks</a>`)
|
assert.Contains(t, body, `class="btn-text">Webhooks</a>`)
|
||||||
assert.Contains(
|
assert.Contains(
|
||||||
t, body, `class="btn-text w-full text-left">Webhooks</a>`,
|
t, body, `class="btn-text w-full text-left">Webhooks</a>`,
|
||||||
@@ -104,7 +121,7 @@ func TestEditPageUsesWebhookTerminology(t *testing.T) {
|
|||||||
// addressable, so a value here renders an error instead of the
|
// addressable, so a value here renders an error instead of the
|
||||||
// page.
|
// page.
|
||||||
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
|
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
|
||||||
webhook.ID = "wh-1"
|
webhook.ID = testWebhookID
|
||||||
|
|
||||||
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
|
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
|
||||||
dataKeyWebhook: webhook,
|
dataKeyWebhook: webhook,
|
||||||
@@ -171,7 +188,7 @@ func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
|
|||||||
t.Cleanup(app.RequireStop)
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
finite := &database.Webhook{Name: "wh", RetentionDays: 14}
|
finite := &database.Webhook{Name: "wh", RetentionDays: 14}
|
||||||
finite.ID = "wh-1"
|
finite.ID = testWebhookID
|
||||||
|
|
||||||
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
|
body := renderPage(t, h, sess, "source_edit.html", map[string]any{
|
||||||
dataKeyWebhook: finite,
|
dataKeyWebhook: finite,
|
||||||
@@ -207,6 +224,16 @@ func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
|
|||||||
t, foreverBody, "Currently forever.",
|
t, foreverBody, "Currently forever.",
|
||||||
"a retain-forever webhook must not read as a day count",
|
"a retain-forever webhook must not read as a day count",
|
||||||
)
|
)
|
||||||
|
assert.Contains(
|
||||||
|
t, foreverBody,
|
||||||
|
"No events are deleted while retention is set to forever",
|
||||||
|
)
|
||||||
|
assert.NotContains(
|
||||||
|
t, foreverBody,
|
||||||
|
"permanently deletes events older than this",
|
||||||
|
"the reaper skips retain-forever webhooks, so the form "+
|
||||||
|
"must not claim it deletes their events",
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestEntrypointCopyButtonIsProgressiveEnhancement proves the copy
|
// TestEntrypointCopyButtonIsProgressiveEnhancement proves the copy
|
||||||
@@ -228,10 +255,23 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
|||||||
entrypoint := database.Entrypoint{Path: "abc123"}
|
entrypoint := database.Entrypoint{Path: "abc123"}
|
||||||
entrypoint.ID = "ep-1"
|
entrypoint.ID = "ep-1"
|
||||||
|
|
||||||
|
// The webhook goes in as a pointer because source_detail.html
|
||||||
|
// calls Webhook.RetentionLabel, a pointer method: a map element
|
||||||
|
// is not addressable, so a value here aborts execution partway
|
||||||
|
// down the page, after the copy button has already been flushed
|
||||||
|
// to the response.
|
||||||
|
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
|
||||||
|
webhook.ID = testWebhookID
|
||||||
|
webhook.CreatedAt = time.Date(
|
||||||
|
2026, time.January, 2, 3, 4, 5, 0, time.UTC,
|
||||||
|
)
|
||||||
|
|
||||||
body := renderPage(t, h, sess, "source_detail.html", map[string]any{
|
body := renderPage(t, h, sess, "source_detail.html", map[string]any{
|
||||||
"Webhook": database.Webhook{Name: "wh"},
|
dataKeyWebhook: webhook,
|
||||||
"Entrypoints": []database.Entrypoint{entrypoint},
|
"Entrypoints": []database.Entrypoint{entrypoint},
|
||||||
"Targets": []database.Target{},
|
// The handler passes delivery.NewTargetViews(targets), never
|
||||||
|
// raw targets, so the test data has to have that same shape.
|
||||||
|
"Targets": delivery.NewTargetViews(nil),
|
||||||
"Events": []database.Event{},
|
"Events": []database.Event{},
|
||||||
"BaseURL": "https://hooks.example.com",
|
"BaseURL": "https://hooks.example.com",
|
||||||
})
|
})
|
||||||
@@ -246,4 +286,14 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
|||||||
`hidden data-copy-target="entrypoint-url-ep-1"`,
|
`hidden data-copy-target="entrypoint-url-ep-1"`,
|
||||||
"the button must start hidden and be revealed by script",
|
"the button must start hidden and be revealed by script",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// renderTemplate streams to the ResponseWriter, so an abort
|
||||||
|
// midway still leaves everything above it in the body. This pins
|
||||||
|
// content from the last line of the template, which is below the
|
||||||
|
// assertions above: without it, a page that renders the copy
|
||||||
|
// button and then 500s passes.
|
||||||
|
assert.Contains(
|
||||||
|
t, body, "Retention: 14 days",
|
||||||
|
"the page must render to completion, not abort partway",
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,7 +29,7 @@
|
|||||||
<div class="form-group">
|
<div class="form-group">
|
||||||
<label for="retention_days" class="label">Retention (days)</label>
|
<label for="retention_days" class="label">Retention (days)</label>
|
||||||
<input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
|
<input type="number" id="retention_days" name="retention_days" value="{{.Webhook.RetentionDays}}" min="0" class="input">
|
||||||
<p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}. A periodic cleanup permanently deletes events older than this, along with their delivery records. Enter 0 to retain events forever; leave blank to keep the current setting.</p>
|
<p class="text-xs text-gray-500 mt-1">Currently {{.Webhook.RetentionLabel}}.{{if .Webhook.RetainsForever}} No events are deleted while retention is set to forever.{{else}} A periodic cleanup permanently deletes events older than this, along with their delivery records.{{end}} Enter 0 to retain events forever; leave blank to keep the current setting.</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div class="flex gap-3">
|
<div class="flex gap-3">
|
||||||
|
|||||||
Reference in New Issue
Block a user