fix(ebooks): harden OPF sidecar ingestion
Close the symlink-swap window in the sidecar reader: os.Open follows symlinks, so a leaf swapped between the Lstat gate and the open could pull metadata from outside the library root. Reject unless the opened handle is the exact file Lstat inspected, matching the image cache processor guard. Replace a stale ISBN provider id on rescan instead of silently dropping it, so sidecar ISBN corrections actually take effect; tolerate the ISBN already belonging to another item so duplicate copies keep scanning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
44db85457c
commit
fcd7eb697d
@@ -117,6 +117,13 @@ func ebookOPFSidecarPath(ebookPath string) string {
|
||||
|
||||
func parseEbookOPFSidecar(sidecarPath string) (parsedEbook, error) {
|
||||
var book parsedEbook
|
||||
leafInfo, err := os.Lstat(sidecarPath)
|
||||
if err != nil {
|
||||
return book, fmt.Errorf("stat ebook OPF sidecar %s: %w", sidecarPath, err)
|
||||
}
|
||||
if !leafInfo.Mode().IsRegular() {
|
||||
return book, fmt.Errorf("ebook OPF sidecar is not a regular file: %s", sidecarPath)
|
||||
}
|
||||
file, err := os.Open(sidecarPath)
|
||||
if err != nil {
|
||||
return book, fmt.Errorf("open ebook OPF sidecar %s: %w", sidecarPath, err)
|
||||
@@ -127,6 +134,13 @@ func parseEbookOPFSidecar(sidecarPath string) (parsedEbook, error) {
|
||||
if err != nil {
|
||||
return book, fmt.Errorf("stat ebook OPF sidecar %s: %w", sidecarPath, err)
|
||||
}
|
||||
// Close the symlink-swap window: os.Open follows symlinks, so a leaf swapped
|
||||
// to a symlink between the Lstat above and this Open would be followed to its
|
||||
// target. Reject unless the opened handle is the exact file Lstat inspected,
|
||||
// so metadata can never be ingested from outside the library root.
|
||||
if !os.SameFile(leafInfo, info) {
|
||||
return book, fmt.Errorf("ebook OPF sidecar is not a regular file: %s", sidecarPath)
|
||||
}
|
||||
if info.Size() > maxEPUBMetadataEntrySize {
|
||||
return book, fmt.Errorf("ebook OPF sidecar too large: %s", sidecarPath)
|
||||
}
|
||||
|
||||
@@ -1304,8 +1304,17 @@ func insertEbookISBNProviderID(ctx context.Context, exec ebookSQLExecutor, conte
|
||||
_, err := exec.Exec(ctx, `
|
||||
INSERT INTO media_item_provider_ids (content_id, provider, provider_id, item_type)
|
||||
VALUES ($1, 'isbn', $2, 'ebook')
|
||||
ON CONFLICT DO NOTHING
|
||||
ON CONFLICT (content_id, provider) DO UPDATE SET
|
||||
provider_id = EXCLUDED.provider_id,
|
||||
updated_at = NOW()
|
||||
WHERE media_item_provider_ids.provider_id IS DISTINCT FROM EXCLUDED.provider_id
|
||||
`, contentID, isbn)
|
||||
// A different item may already own this ISBN under the (provider, provider_id)
|
||||
// unique constraint; duplicate copies must not fail the scan.
|
||||
var pgErr *pgconn.PgError
|
||||
if errors.As(err, &pgErr) && pgErr.Code == "23505" {
|
||||
return nil
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@@ -372,6 +372,62 @@ func TestParseEbookFileAppliesExternalOPFSidecar(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseEbookOPFSidecarRejectsSymlinkedSidecar(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
target := filepath.Join(dir, "outside-library.opf")
|
||||
sidecar := `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<package xmlns:dc="http://purl.org/dc/elements/1.1/">
|
||||
<metadata><dc:title>Smuggled</dc:title></metadata>
|
||||
</package>`
|
||||
if err := os.WriteFile(target, []byte(sidecar), 0o644); err != nil {
|
||||
t.Fatalf("write target: %v", err)
|
||||
}
|
||||
link := filepath.Join(dir, "Dune.opf")
|
||||
if err := os.Symlink(target, link); err != nil {
|
||||
t.Fatalf("symlink: %v", err)
|
||||
}
|
||||
|
||||
if _, err := parseEbookOPFSidecar(link); err == nil {
|
||||
t.Fatal("parseEbookOPFSidecar accepted a symlinked sidecar; a leaf swapped to a symlink after the Lstat gate would be followed to its target")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInsertEbookISBNProviderIDReplacesStaleISBN(t *testing.T) {
|
||||
exec := &recordingEbookExecutor{}
|
||||
if err := insertEbookISBNProviderID(context.Background(), exec, "book-1", "9780441172719"); err != nil {
|
||||
t.Fatalf("insertEbookISBNProviderID: %v", err)
|
||||
}
|
||||
if len(exec.queries) != 1 {
|
||||
t.Fatalf("queries = %d, want 1", len(exec.queries))
|
||||
}
|
||||
query := strings.Join(strings.Fields(exec.queries[0]), " ")
|
||||
if !strings.Contains(query, "ON CONFLICT (content_id, provider) DO UPDATE") ||
|
||||
!strings.Contains(query, "provider_id = EXCLUDED.provider_id") {
|
||||
t.Fatalf("ISBN insert does not replace a stale value on rescan:\n%s", query)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInsertEbookISBNProviderIDToleratesISBNOwnedByAnotherItem(t *testing.T) {
|
||||
exec := &erroringEbookExecutor{err: &pgconn.PgError{Code: "23505"}}
|
||||
if err := insertEbookISBNProviderID(context.Background(), exec, "book-1", "9780441172719"); err != nil {
|
||||
t.Fatalf("unique-violation on the (provider, provider_id) constraint must not fail the scan, got %v", err)
|
||||
}
|
||||
|
||||
boom := errors.New("connection lost")
|
||||
exec = &erroringEbookExecutor{err: boom}
|
||||
if err := insertEbookISBNProviderID(context.Background(), exec, "book-1", "9780441172719"); !errors.Is(err, boom) {
|
||||
t.Fatalf("non-unique-violation error = %v, want %v", err, boom)
|
||||
}
|
||||
}
|
||||
|
||||
type erroringEbookExecutor struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (e *erroringEbookExecutor) Exec(_ context.Context, _ string, _ ...any) (pgconn.CommandTag, error) {
|
||||
return pgconn.CommandTag{}, e.err
|
||||
}
|
||||
|
||||
func TestParseEbookEPUBMetadataAllowsXMLVersion11(t *testing.T) {
|
||||
path := writeTestEPUBWithOPFBytes(t, []byte(`<?xml version="1.1" encoding="UTF-8"?>
|
||||
<package xmlns:dc="http://purl.org/dc/elements/1.1/"><metadata>
|
||||
|
||||
Reference in New Issue
Block a user