RomanNum3ral
/
Cybersecurity-Projects
mirror of https://github.com/CarterPerez-dev/Cybersecurity-Projects.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(canary-phase2): address audit findings before rollup 

Two audit agents (code-reviewer + spec-adherence) both returned PASS but
flagged substantive findings. Per fix-in-phase / no-backlog-rot, clearing
every MEDIUM + LOW in-phase.

Findings addressed:

MEDIUM — realIP RemoteAddr fallback returned "IP:port" verbatim, where
the spec wants the bare IP (geoip + downstream parsing assume host-only).
Now uses net.SplitHostPort with raw-string fallback if not host:port.
Adds 4 RemoteAddr cases: IPv4 strips port, IPv6 bracket form strips
brackets+port, loopback IPv6, and the port-less raw fallback.

MEDIUM — realIP XFF branch accepted whatever rightmost-comma-split
produced, so headers like "198.51.100.1, " (trailing comma) yielded
"" and skipped XRI/RemoteAddr entirely. Extracted into lastNonEmptyXFF
which walks right-to-left and falls through cleanly. Adds two cases:
trailing-comma falls through, all-empty entries fall through.

MEDIUM — pixel.TransparentGIF was an exported mutable []byte; any caller
could clobber it process-wide (mutating one response's body would affect
every subsequent webbug trigger). Renamed to unexported transparentGIF
+ exposed pixel.Clone() and pixel.Len(). Webbug now calls pixel.Clone()
per trigger; new test TestTrigger_ResponseBodyIsIndependentCopyPerCall
verifies two triggers return independent slices.

LOW (reviewer-escalated to HIGH-for-Phase-3) — Trigger on nil token
returned a non-nil event with empty TokenID. Since events.token_id is a
NOT NULL FK to tokens.id, the Phase 3 handler could not have persisted
that event anyway, and the contract was implicit (would have required
an inline comment, which the no-comments rule forbids). Now: nil token
in → nil event, non-nil response out. Contract is explicit in the
return shape. Test asserts evt is nil for nil-token path.

LOW — no IPv6 coverage in IP-precedence tests. Added IPv6 cases for
XFF rightmost and three RemoteAddr forms (bracketed, loopback, no-port).

NIT — gracefulShutdown swallowed every shutdown error, returning nil
unconditionally. Now collects with errors.Join so callers can detect
partial-shutdown for telemetry/exit-code purposes.

Audits accepted as-is (NIT, not blocking):
- Artifact discriminated-union shape (Phase 3+ concern when other Kinds
  are produced)
- Registry returning bare map (read-only post-Build; concurrent reads
  are safe by Go's memory model)
- Build(_ Config) ignoring its arg (signature reserved for future
  stateful generators)

Quality verified post-fix: build/vet/lint clean (0 issues),
14 webbug tests + 6 pixel tests + 4 registry tests PASS under -race,
integration tests unchanged.
This commit is contained in:
CarterPerez-dev 2026-05-12 02:56:29 -04:00
parent c39b56b9af
commit 1a240952c7
5 changed files with 210 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
PROJECTS/beginner/canary-token-generator/backend/cmd/canary/main.go
View File

@ -5,6 +5,7 @@ package main
import (
"context"
"errors"
"flag"
"fmt"
"log/slog"
@ -159,23 +160,28 @@ func gracefulShutdown(
)
defer cancel()
var errs []error
if err := srv.Shutdown(shutdownCtx, drainDelay); err != nil {
logger.Error("server shutdown error", "error", err)
errs = append(errs, fmt.Errorf("server shutdown: %w", err))
}
if telemetry != nil {
if err := telemetry.Shutdown(shutdownCtx); err != nil {
logger.Error("telemetry shutdown error", "error", err)
errs = append(errs, fmt.Errorf("telemetry shutdown: %w", err))
}
}
if err := rdb.Close(); err != nil {
logger.Error("redis close error", "error", err)
errs = append(errs, fmt.Errorf("redis close: %w", err))
}
if err := db.Close(); err != nil {
logger.Error("database close error", "error", err)
errs = append(errs, fmt.Errorf("database close: %w", err))
}
logger.Info("application stopped")
return nil
return errors.Join(errs...)
}
func setupLogger(cfg config.LogConfig) *slog.Logger {

12
PROJECTS/beginner/canary-token-generator/backend/internal/token/generators/pixel/pixel.go
View File

@ -3,7 +3,9 @@
package pixel
var TransparentGIF = []byte{
import "slices"
var transparentGIF = []byte{
0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00, 0x01, 0x00, 0x80, 0x00,
0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0x21, 0xf9, 0x04, 0x01, 0x00,
0x00, 0x00, 0x00, 0x2c, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00,
@ -11,3 +13,11 @@ var TransparentGIF = []byte{
}
const ContentType = "image/gif"
func Clone() []byte {
return slices.Clone(transparentGIF)
}
func Len() int {
return len(transparentGIF)
}

41
PROJECTS/beginner/canary-token-generator/backend/internal/token/generators/pixel/pixel_test.go
View File

@ -24,27 +24,32 @@ var (
gifTrailer = byte(0x3b)
)
func TestTransparentGIF_Length(t *testing.T) {
require.Len(t, pixel.TransparentGIF, expectedLength)
func TestLen_Is43(t *testing.T) {
require.Equal(t, expectedLength, pixel.Len())
}
func TestTransparentGIF_MagicBytes(t *testing.T) {
func TestClone_Length(t *testing.T) {
require.Len(t, pixel.Clone(), expectedLength)
}
func TestClone_MagicBytes(t *testing.T) {
g := pixel.Clone()
require.True(
t,
bytes.HasPrefix(pixel.TransparentGIF, gif89aMagic),
bytes.HasPrefix(g, gif89aMagic),
"expected GIF89a magic prefix, got % x",
pixel.TransparentGIF[:len(gif89aMagic)],
g[:len(gif89aMagic)],
)
require.Equal(
t,
gifTrailer,
pixel.TransparentGIF[len(pixel.TransparentGIF)-1],
g[len(g)-1],
"expected trailing GIF terminator 0x3B",
)
}
func TestTransparentGIF_DecodesAsImageGIF(t *testing.T) {
img, err := gif.Decode(bytes.NewReader(pixel.TransparentGIF))
func TestClone_DecodesAsImageGIF(t *testing.T) {
img, err := gif.Decode(bytes.NewReader(pixel.Clone()))
require.NoError(t, err)
require.NotNil(t, img)
@ -53,6 +58,26 @@ func TestTransparentGIF_DecodesAsImageGIF(t *testing.T) {
require.Equal(t, expectedHeight, bounds.Dy())
}
func TestClone_ReturnsIndependentCopy(t *testing.T) {
a := pixel.Clone()
b := pixel.Clone()
require.Equal(t, a, b, "two clones must be byte-equal")
a[0] = 0x00
require.Equal(
t,
byte(0x47),
b[0],
"mutating one clone must not affect another",
)
require.Equal(
t,
byte(0x47),
pixel.Clone()[0],
"mutating one clone must not affect the package-internal source",
)
}
func TestContentType_IsImageGIF(t *testing.T) {
require.Equal(t, "image/gif", pixel.ContentType)
}

45
PROJECTS/beginner/canary-token-generator/backend/internal/token/generators/webbug/generator.go
View File

@ -5,6 +5,7 @@ package webbug
import (
"context"
"net"
"net/http"
"strings"
@ -50,25 +51,26 @@ func (g *Generator) Trigger(
t *token.Token,
r *http.Request,
) (*event.Event, *generators.TriggerResponse, error) {
tokenID := ""
if t != nil {
tokenID = t.ID
}
evt := &event.Event{
TokenID: tokenID,
SourceIP: realIP(r),
UserAgent: optionalHeader(r.UserAgent()),
Referer: optionalHeader(r.Header.Get(headerReferer)),
}
resp := &generators.TriggerResponse{
StatusCode: http.StatusOK,
ContentType: pixel.ContentType,
Body: pixel.TransparentGIF,
Body: pixel.Clone(),
ExtraHeaders: map[string]string{
headerCacheControl: cacheControlNoStore,
headerPragma: pragmaNoCache,
},
}
if t == nil {
return nil, resp, nil
}
evt := &event.Event{
TokenID: t.ID,
SourceIP: realIP(r),
UserAgent: optionalHeader(r.UserAgent()),
Referer: optionalHeader(r.Header.Get(headerReferer)),
}
return evt, resp, nil
}
@ -84,12 +86,27 @@ func realIP(r *http.Request) string {
if v := strings.TrimSpace(r.Header.Get(headerCFConnectingIP)); v != "" {
return v
}
if v := r.Header.Get(headerXForwardedFor); v != "" {
parts := strings.Split(v, ",")
return strings.TrimSpace(parts[len(parts)-1])
if v := lastNonEmptyXFF(r.Header.Get(headerXForwardedFor)); v != "" {
return v
}
if v := strings.TrimSpace(r.Header.Get(headerXRealIP)); v != "" {
return v
}
if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
return host
}
return r.RemoteAddr
}
func lastNonEmptyXFF(header string) string {
if header == "" {
return ""
}
parts := strings.Split(header, ",")
for i := len(parts) - 1; i >= 0; i-- {
if v := strings.TrimSpace(parts[i]); v != "" {
return v
}
}
return ""
}

203
PROJECTS/beginner/canary-token-generator/backend/internal/token/generators/webbug/generator_test.go
View File

@ -106,72 +106,114 @@ func TestTrigger_RecordsEventWithRequestMetadata(t *testing.T) {
},
)
t.Run(
"source ip precedence CF > XFF (last) > XRI > RemoteAddr",
func(t *testing.T) {
cases := []struct {
name string
headers map[string]string
remote string
wantIP string
}{
{
name: "CF wins over XFF and XRI",
headers: map[string]string{
"CF-Connecting-IP": "203.0.113.10",
"X-Forwarded-For": "198.51.100.1, 198.51.100.2",
"X-Real-IP": "192.0.2.99",
},
remote: "127.0.0.1:9999",
wantIP: "203.0.113.10",
t.Run("source ip precedence", func(t *testing.T) {
cases := []struct {
name string
headers map[string]string
remote string
wantIP string
}{
{
name: "CF wins over XFF and XRI",
headers: map[string]string{
"CF-Connecting-IP": "203.0.113.10",
"X-Forwarded-For": "198.51.100.1, 198.51.100.2",
"X-Real-IP": "192.0.2.99",
},
{
name: "XFF rightmost wins over XRI when no CF",
headers: map[string]string{
"X-Forwarded-For": "198.51.100.1, 198.51.100.7",
"X-Real-IP": "192.0.2.99",
},
remote: "127.0.0.1:9999",
wantIP: "198.51.100.7",
remote: "127.0.0.1:9999",
wantIP: "203.0.113.10",
},
{
name: "XFF rightmost wins over XRI when no CF",
headers: map[string]string{
"X-Forwarded-For": "198.51.100.1, 198.51.100.7",
"X-Real-IP": "192.0.2.99",
},
{
name: "XRI when no CF or XFF",
headers: map[string]string{
"X-Real-IP": "192.0.2.99",
},
remote: "127.0.0.1:9999",
wantIP: "192.0.2.99",
remote: "127.0.0.1:9999",
wantIP: "198.51.100.7",
},
{
name: "XFF trailing-comma falls through to XRI",
headers: map[string]string{
"X-Forwarded-For": "198.51.100.1, ",
"X-Real-IP": "192.0.2.99",
},
{
name: "RemoteAddr when no proxy headers",
headers: nil,
remote: "127.0.0.1:9999",
wantIP: "127.0.0.1:9999",
remote: "127.0.0.1:9999",
wantIP: "198.51.100.1",
},
{
name: "XFF entirely empty entries fall through to XRI",
headers: map[string]string{
"X-Forwarded-For": ", ,",
"X-Real-IP": "192.0.2.99",
},
{
name: "CF value is trimmed of whitespace",
headers: map[string]string{
"CF-Connecting-IP": " 203.0.113.10 ",
},
remote: "127.0.0.1:9999",
wantIP: "203.0.113.10",
remote: "127.0.0.1:9999",
wantIP: "192.0.2.99",
},
{
name: "XRI when no CF or XFF",
headers: map[string]string{
"X-Real-IP": "192.0.2.99",
},
}
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/c/token1", nil)
for k, v := range tc.headers {
r.Header.Set(k, v)
}
r.RemoteAddr = tc.remote
evt, _, err := g.Trigger(context.Background(), tok, r)
require.NoError(t, err)
require.Equal(t, tc.wantIP, evt.SourceIP)
})
}
},
)
remote: "127.0.0.1:9999",
wantIP: "192.0.2.99",
},
{
name: "RemoteAddr IPv4 strips port",
headers: nil,
remote: "127.0.0.1:9999",
wantIP: "127.0.0.1",
},
{
name: "RemoteAddr IPv6 strips brackets and port",
headers: nil,
remote: "[2001:db8::1]:54321",
wantIP: "2001:db8::1",
},
{
name: "RemoteAddr loopback IPv6 strips brackets and port",
headers: nil,
remote: "[::1]:9999",
wantIP: "::1",
},
{
name: "RemoteAddr without port falls back to raw value",
headers: nil,
remote: "127.0.0.1",
wantIP: "127.0.0.1",
},
{
name: "XFF IPv6 rightmost",
headers: map[string]string{
"X-Forwarded-For": "198.51.100.1, 2001:db8::dead",
},
remote: "127.0.0.1:9999",
wantIP: "2001:db8::dead",
},
{
name: "CF value is trimmed of whitespace",
headers: map[string]string{
"CF-Connecting-IP": " 203.0.113.10 ",
},
remote: "127.0.0.1:9999",
wantIP: "203.0.113.10",
},
}
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/c/token1", nil)
for k, v := range tc.headers {
r.Header.Set(k, v)
}
r.RemoteAddr = tc.remote
evt, _, err := g.Trigger(context.Background(), tok, r)
require.NoError(t, err)
require.NotNil(t, evt)
require.Equal(t, tc.wantIP, evt.SourceIP)
})
}
})
t.Run(
"missing user agent and referer record as nil pointers",
@ -183,6 +225,7 @@ func TestTrigger_RecordsEventWithRequestMetadata(t *testing.T) {
evt, _, err := g.Trigger(context.Background(), tok, r)
require.NoError(t, err)
require.NotNil(t, evt)
require.Nil(
t,
evt.UserAgent,
@ -208,11 +251,30 @@ func TestTrigger_ResponseIs43ByteGIF(t *testing.T) {
require.Equal(t, http.StatusOK, resp.StatusCode)
require.Equal(t, pixel.ContentType, resp.ContentType)
require.Len(t, resp.Body, gifByteLength)
require.Equal(t, pixel.TransparentGIF, resp.Body)
require.Equal(t, pixel.Clone(), resp.Body)
require.Equal(t, cacheControlNoStore, resp.ExtraHeaders["Cache-Control"])
require.Equal(t, pragmaNoCache, resp.ExtraHeaders["Pragma"])
}
func TestTrigger_ResponseBodyIsIndependentCopyPerCall(t *testing.T) {
g := webbug.New()
tok := newWebbugToken("abc")
r := httptest.NewRequest(http.MethodGet, "/c/abc", nil)
_, resp1, err := g.Trigger(context.Background(), tok, r)
require.NoError(t, err)
_, resp2, err := g.Trigger(context.Background(), tok, r)
require.NoError(t, err)
resp1.Body[0] = 0x00
require.Equal(
t,
byte(0x47),
resp2.Body[0],
"each Trigger call must produce an independent body slice",
)
}
func TestTrigger_TokenNotFound_StillReturnsGIF(t *testing.T) {
g := webbug.New()
r := httptest.NewRequest(http.MethodGet, "/c/does-not-exist", nil)
@ -228,19 +290,10 @@ func TestTrigger_TokenNotFound_StillReturnsGIF(t *testing.T) {
require.NotNil(t, resp, "nil-token path must still return GIF response")
require.Equal(t, http.StatusOK, resp.StatusCode)
require.Equal(t, pixel.ContentType, resp.ContentType)
require.Equal(t, pixel.TransparentGIF, resp.Body)
require.NotNil(t, evt, "event value still produced for forensic continuity")
require.Empty(
require.Equal(t, pixel.Clone(), resp.Body)
require.Nil(
t,
evt.TokenID,
"empty TokenID signals nil token; persistence layer decides what to do",
evt,
"nil-token path returns nil event so the handler cannot accidentally persist a row with empty TokenID (FK violation)",
)
require.Equal(
t,
"203.0.113.100",
evt.SourceIP,
"source IP still captured for forensics",
)
require.NotNil(t, evt.UserAgent)
require.Equal(t, "curl/8.0.0", *evt.UserAgent)
}