fix(canary): PDF trigger URL uses ?p= query pad; handler trims trailing _
Audit F2: the PDF placeholder was padded with underscores embedded directly in the /URI path, so Acrobat fetched /c/<id>____________ and the chi route extracted the padded id. Lookup returned ErrNotFound, the webbug fallback served a 1x1 GIF, and event recording skipped because tok was nil. PDF tokens silently never recorded events. Part B (durable): pad inside a ?p= query string so the trigger path stays canonical and chi extracts the real id. Part A (defense in depth): handler TrimRight on _ so PDFs already in the field also resolve correctly. Added two regression tests: byte-after-id assertion in the PDF artifact, and the handler's trim path.
This commit is contained in:
parent
4382ba5e17
commit
30ebda39d5
|
|
@ -35,6 +35,8 @@ const (
|
||||||
|
|
||||||
padChar = "_"
|
padChar = "_"
|
||||||
|
|
||||||
|
queryPadPrefix = "?p="
|
||||||
|
|
||||||
contentType = "application/pdf"
|
contentType = "application/pdf"
|
||||||
defaultFilename = "Document.pdf"
|
defaultFilename = "Document.pdf"
|
||||||
)
|
)
|
||||||
|
|
@ -71,8 +73,7 @@ func (g *Generator) Generate(
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
padded := triggerURL +
|
padded := padTriggerURL(triggerURL)
|
||||||
strings.Repeat(padChar, PlaceholderLength-len(triggerURL))
|
|
||||||
|
|
||||||
out := bytes.Replace(
|
out := bytes.Replace(
|
||||||
pdfTemplate,
|
pdfTemplate,
|
||||||
|
|
@ -140,3 +141,16 @@ func resolveFilename(name *string) string {
|
||||||
}
|
}
|
||||||
return trimmed
|
return trimmed
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func padTriggerURL(triggerURL string) string {
|
||||||
|
needed := PlaceholderLength - len(triggerURL)
|
||||||
|
switch {
|
||||||
|
case needed == 0:
|
||||||
|
return triggerURL
|
||||||
|
case needed >= len(queryPadPrefix):
|
||||||
|
return triggerURL + queryPadPrefix +
|
||||||
|
strings.Repeat(padChar, needed-len(queryPadPrefix))
|
||||||
|
default:
|
||||||
|
return triggerURL + strings.Repeat(padChar, needed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -286,6 +286,34 @@ func TestGenerate_ContainsTriggerURL(t *testing.T) {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGenerate_ByteAfterTokenIDIsNotUnderscore(t *testing.T) {
|
||||||
|
g := pdf.New()
|
||||||
|
art, err := g.Generate(
|
||||||
|
context.Background(),
|
||||||
|
newPDFToken("token42"),
|
||||||
|
testBaseURL,
|
||||||
|
)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
urlPrefix := testBaseURL + "/c/token42"
|
||||||
|
idx := bytes.Index(art.Content, []byte(urlPrefix))
|
||||||
|
require.GreaterOrEqual(
|
||||||
|
t,
|
||||||
|
idx,
|
||||||
|
0,
|
||||||
|
"trigger URL prefix must be present in PDF output",
|
||||||
|
)
|
||||||
|
after := art.Content[idx+len(urlPrefix)]
|
||||||
|
require.NotEqual(
|
||||||
|
t,
|
||||||
|
byte('_'),
|
||||||
|
after,
|
||||||
|
"byte immediately after the token id must not be underscore — "+
|
||||||
|
"otherwise Acrobat fetches /c/token42____ and the canary silently "+
|
||||||
|
"no-ops on lookup (audit finding F2)",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
func TestGenerate_PlaceholderRemoved(t *testing.T) {
|
func TestGenerate_PlaceholderRemoved(t *testing.T) {
|
||||||
g := pdf.New()
|
g := pdf.New()
|
||||||
art, err := g.Generate(
|
art, err := g.Generate(
|
||||||
|
|
|
||||||
|
|
@ -162,7 +162,7 @@ func (h *Handler) CreateToken(w http.ResponseWriter, r *http.Request) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func (h *Handler) HandleTrigger(w http.ResponseWriter, r *http.Request) {
|
func (h *Handler) HandleTrigger(w http.ResponseWriter, r *http.Request) {
|
||||||
id := chi.URLParam(r, urlParamTokenID)
|
id := strings.TrimRight(chi.URLParam(r, urlParamTokenID), "_")
|
||||||
if id == "" {
|
if id == "" {
|
||||||
http.NotFound(w, r)
|
http.NotFound(w, r)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -220,6 +220,40 @@ func TestHandleTrigger_KnownTokenReturnsResponseAndRecordsEvent(t *testing.T) {
|
||||||
require.Len(t, rec.events, 1)
|
require.Len(t, rec.events, 1)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestHandleTrigger_TrimsTrailingUnderscoresFromPaddedID(t *testing.T) {
|
||||||
|
gen := &triggerGen{
|
||||||
|
tokenType: token.TypeWebbug,
|
||||||
|
artifact: generators.Artifact{Kind: generators.KindURL},
|
||||||
|
resp: &generators.TriggerResponse{
|
||||||
|
StatusCode: 200,
|
||||||
|
ContentType: "image/gif",
|
||||||
|
Body: []byte{0x47},
|
||||||
|
},
|
||||||
|
evt: &event.Event{SourceIP: "1.2.3.4"},
|
||||||
|
}
|
||||||
|
h, repo, rec := newWebbugHandler(t, gen)
|
||||||
|
|
||||||
|
tok := &token.Token{
|
||||||
|
ID: "abcdef012345", ManageID: "m", Type: token.TypeWebbug,
|
||||||
|
AlertChannel: token.ChannelWebhook, Enabled: true,
|
||||||
|
Metadata: json.RawMessage(`{}`),
|
||||||
|
}
|
||||||
|
require.NoError(t, repo.Insert(context.Background(), tok))
|
||||||
|
|
||||||
|
r := chi.NewRouter()
|
||||||
|
h.RegisterTriggerRoutes(r)
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
padded := "/c/abcdef012345____________________"
|
||||||
|
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, padded, nil))
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
require.Len(t, rec.events, 1,
|
||||||
|
"trailing-underscore padded id must still resolve to the real token "+
|
||||||
|
"so PDFs already in the field (carrying padded URLs) record events "+
|
||||||
|
"instead of silently no-opping (audit finding F2)")
|
||||||
|
}
|
||||||
|
|
||||||
func TestHandleTrigger_UnknownTokenStillReturnsArtifactShape(t *testing.T) {
|
func TestHandleTrigger_UnknownTokenStillReturnsArtifactShape(t *testing.T) {
|
||||||
gen := &triggerGen{
|
gen := &triggerGen{
|
||||||
tokenType: token.TypeWebbug,
|
tokenType: token.TypeWebbug,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue