diff --git a/CHANGELOG.md b/CHANGELOG.md index ce15229..74b0a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.11.8 - Unreleased + +### Fixes + +- Keep attachment fetches compatible with three allowed Discord CDN redirects and injected HTTP transports while preserving final-response host validation. +- Reject malformed tombstone timestamps before routine incremental snapshot imports mutate the archive. + ## 0.11.7 - 2026-07-17 ### Highlights diff --git a/internal/media/cache.go b/internal/media/cache.go index f480c66..e18724b 100644 --- a/internal/media/cache.go +++ b/internal/media/cache.go @@ -138,7 +138,7 @@ func attachmentHTTPClient(client *http.Client) *http.Client { clone := *client previous := clone.CheckRedirect clone.CheckRedirect = func(req *http.Request, via []*http.Request) error { - if len(via) >= 3 || !isAllowedAttachmentURL(req.URL.String()) { + if len(via) > 3 || !isAllowedAttachmentURL(req.URL.String()) { return errors.New("attachment redirect denied") } if previous != nil { @@ -257,7 +257,7 @@ func fetchURL(ctx context.Context, opts FetchOptions, attachment store.Attachmen defer func() { _ = resp.Body.Close() }() // Injected clients can supply their own redirect policy, so validate the // final response URL at the fetch boundary as well. - if resp.Request == nil || resp.Request.URL == nil || !isAllowedAttachmentURL(resp.Request.URL.String()) { + if resp.Request != nil && (resp.Request.URL == nil || !isAllowedAttachmentURL(resp.Request.URL.String())) { return fetchResult{}, errors.New("attachment response URL denied") } if resp.StatusCode < 200 || resp.StatusCode >= 300 { diff --git a/internal/media/cache_test.go b/internal/media/cache_test.go index b951a6f..9d2ea6f 100644 --- a/internal/media/cache_test.go +++ b/internal/media/cache_test.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/hex" "errors" + "fmt" "io" "net/http" "net/url" @@ -59,6 +60,32 @@ func TestFetchCachesAttachmentMedia(t *testing.T) { require.Equal(t, 1, stats.Reused) } +func TestFetchAllowsInjectedRoundTripperWithoutResponseRequest(t *testing.T) { + t.Parallel() + + ctx := context.Background() + s, err := store.Open(ctx, filepath.Join(t.TempDir(), "discrawl.db")) + require.NoError(t, err) + defer func() { _ = s.Close() }() + require.NoError(t, seedAttachment(ctx, s, "https://cdn.discordapp.com/attachments/c1/file.png")) + + body := []byte("image-bytes") + stats, err := Fetch(ctx, s, FetchOptions{ + CacheDir: t.TempDir(), + MaxBytes: 1024, + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(bytes.NewReader(body)), + ContentLength: int64(len(body)), + }, nil + })}, + }) + require.NoError(t, err) + require.Equal(t, FetchStats{Attachments: 1, Fetched: 1, Bytes: int64(len(body))}, stats) +} + func TestFetchLimitAppliesAfterExistingCacheCheck(t *testing.T) { t.Parallel() @@ -526,6 +553,50 @@ func TestAttachmentHTTPClientRejectsRedirectCallbackURLMutation(t *testing.T) { require.EqualValues(t, 1, calls.Load()) } +func TestAttachmentHTTPClientAllowsThreeRedirectsAndRejectsFourth(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + redirects int + wantError bool + }{ + {name: "three redirects", redirects: 3}, + {name: "fourth redirect", redirects: 4, wantError: true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var calls atomic.Int32 + client := attachmentHTTPClient(&http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + call := int(calls.Add(1)) + response := &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("ok")), + Request: req, + } + if call <= tc.redirects { + response.StatusCode = http.StatusFound + response.Header.Set("Location", fmt.Sprintf("/attachments/c/%d", call)) + } + return response, nil + })}) + request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://cdn.discordapp.com/attachments/c/0", nil) + require.NoError(t, err) + response, err := client.Do(request) + if response != nil { + _ = response.Body.Close() + } + if tc.wantError { + require.ErrorContains(t, err, "attachment redirect denied") + } else { + require.NoError(t, err) + } + require.EqualValues(t, 4, calls.Load()) + }) + } +} + func TestMediaTargetNeedsWrite(t *testing.T) { t.Parallel() diff --git a/internal/share/share.go b/internal/share/share.go index 03d57c5..5638e07 100644 --- a/internal/share/share.go +++ b/internal/share/share.go @@ -658,7 +658,13 @@ func importMergePlan( }) }, Filter: func(table string, row map[string]any) (bool, error) { - return !isDirectMessageSnapshotRow(table, row), nil + if isDirectMessageSnapshotRow(table, row) { + return false, nil + } + if err := validateSnapshotRow(table, row); err != nil { + return false, err + } + return true, nil }, BeforeImport: func(ctx context.Context, tx *sql.Tx) error { var err error diff --git a/internal/share/share_test.go b/internal/share/share_test.go index b4921aa..c93df91 100644 --- a/internal/share/share_test.go +++ b/internal/share/share_test.go @@ -937,6 +937,43 @@ func TestMergeIfChangedUsesIncrementalTailImport(t *testing.T) { require.Contains(t, state, `"file_manifests"`) } +func TestMergeIfChangedRejectsMalformedTombstoneBeforeIncrementalMutation(t *testing.T) { + t.Parallel() + + ctx := context.Background() + src := seedStore(t, filepath.Join(t.TempDir(), "src.db")) + defer func() { _ = src.Close() }() + + repo := filepath.Join(t.TempDir(), "share") + initial, err := Export(ctx, src, Options{RepoPath: repo, Branch: "main"}) + require.NoError(t, err) + + dst, err := store.Open(ctx, filepath.Join(t.TempDir(), "dst.db")) + require.NoError(t, err) + defer func() { _ = dst.Close() }() + _, changed, err := MergeIfChanged(ctx, dst, Options{RepoPath: repo, Branch: "main"}) + require.NoError(t, err) + require.True(t, changed) + + now := time.Now().UTC().Format(time.RFC3339Nano) + _, err = src.DB().ExecContext(ctx, `update messages set deleted_at = 'not-a-timestamp', updated_at = ? where id = 'm1'`, now) + require.NoError(t, err) + updated, err := Export(ctx, src, Options{RepoPath: repo, Branch: "main"}) + require.NoError(t, err) + require.NotEqual(t, initial.GeneratedAt, updated.GeneratedAt) + + _, changed, err = MergeIfChanged(ctx, dst, Options{RepoPath: repo, Branch: "main"}) + require.ErrorContains(t, err, "messages.deleted_at must be RFC3339") + require.False(t, changed) + + var deletedAt string + require.NoError(t, dst.DB().QueryRowContext(ctx, `select coalesce(deleted_at, '') from messages where id = 'm1'`).Scan(&deletedAt)) + require.Empty(t, deletedAt) + previous, ok := PreviousMergedManifest(ctx, dst, Options{RepoPath: repo, Branch: "main"}) + require.True(t, ok) + require.Equal(t, initial.GeneratedAt, previous.GeneratedAt) +} + func TestMergeIfChangedUsesMixedIncrementalPlanForMetadataChanges(t *testing.T) { ctx := context.Background() src := seedStore(t, filepath.Join(t.TempDir(), "src.db"))