diff --git a/internal/httpapi/notes_handlers.go b/internal/httpapi/notes_handlers.go index cf676d1..ec9f42d 100644 --- a/internal/httpapi/notes_handlers.go +++ b/internal/httpapi/notes_handlers.go @@ -353,7 +353,16 @@ func (s *Server) handleImportNotes(w http.ResponseWriter, r *http.Request) { for _, f := range zr.File { name := f.Name - if f.FileInfo().IsDir() || skipFile(name) || !strings.EqualFold(path.Ext(name), ".md") { + if f.FileInfo().IsDir() || !strings.EqualFold(path.Ext(name), ".md") { + continue + } + // Counted, not passed over in silence. A Markdown file the filter refuses is + // a note that did not arrive, and the caller is told the same number either + // way unless it is said here — which is how one note went missing without + // anything looking wrong. + if skipFile(name) { + s.log.Warn("import skipped a note file", "file", name) + skipped++ continue } @@ -409,10 +418,34 @@ func (s *Server) handleImportNotes(w http.ResponseWriter, r *http.Request) { // A zip made on a Mac carries a __MACOSX copy of every file, and importing those // would double a whole library with each copy full of binary rubbish. func skipFile(name string) bool { - return strings.HasPrefix(name, "__MACOSX/") || + if strings.HasPrefix(name, "__MACOSX/") || strings.HasPrefix(path.Base(name), "._") || - strings.HasPrefix(path.Base(name), ".") || - strings.Contains(name, "..") + strings.HasPrefix(path.Base(name), ".") { + return true + } + + // Traversal, read as path segments rather than as a substring. + // + // This was `strings.Contains(name, "..")`, which is the right idea aimed at the + // wrong thing: it does not describe a path that climbs out of the archive, it + // describes any name with two dots next to each other anywhere in it. A note + // called "Så blev det endelig jul! Stay tuned... ☕️" was therefore dropped — + // silently, and without even counting as skipped, so the number the import + // reported still looked right. One note in twelve hundred, found only by + // counting what came out. + // + // An ellipsis in a title is ordinary. A segment that *is* "..", or a path that + // starts at the root, is not — and those are what a zip uses to write outside + // the folder it was unpacked into. + if strings.HasPrefix(name, "/") { + return true + } + for _, part := range strings.Split(name, "/") { + if part == ".." { + return true + } + } + return false } // attachToNote records the files a note's text points at and rewrites the text to diff --git a/internal/httpapi/notes_import_filter_test.go b/internal/httpapi/notes_import_filter_test.go new file mode 100644 index 0000000..4b3d810 --- /dev/null +++ b/internal/httpapi/notes_import_filter_test.go @@ -0,0 +1,38 @@ +package httpapi + +import "testing" + +// En titel med tre prikker i er ikke et forsøg på at skrive uden for mappen. +// +// `strings.Contains(name, "..")` var den rigtige idé rettet mod det forkerte: +// den beskriver ikke en sti, der klatrer ud af arkivet, men ethvert navn med to +// prikker ved siden af hinanden. Noten "Så blev det endelig jul! Stay tuned... ☕️" +// blev derfor droppet lydløst — én ud af tolv hundrede, og kun fundet ved at +// tælle efter, hvor mange der kom ud. +func TestSkipFileTakesEllipsesButRefusesTraversal(t *testing.T) { + keep := []string{ + "Så blev det endelig jul! Stay tuned... ☕️.md", + "Møde 12.8..md", + "vedhaeftninger/1-2-Pasted Graphic.png", + "noter/Referat....md", + } + for _, name := range keep { + if skipFile(name) { + t.Errorf("skipFile(%q) = true, men det er et almindeligt filnavn", name) + } + } + + drop := []string{ + "../uden for.md", + "noter/../../etc/passwd", + "/etc/passwd", + "__MACOSX/._noget", + ".skjult.md", + "noter/._AppleDouble.md", + } + for _, name := range drop { + if !skipFile(name) { + t.Errorf("skipFile(%q) = false, men den skal afvises", name) + } + } +} diff --git a/web/src/lib/components/Sidebar.svelte b/web/src/lib/components/Sidebar.svelte index 84ca599..d2e63cc 100644 --- a/web/src/lib/components/Sidebar.svelte +++ b/web/src/lib/components/Sidebar.svelte @@ -508,14 +508,22 @@ sidebar is already a drop target for tasks — a task dragged onto I dag stopped landing, because the row had claimed the gesture. A grip claims nothing until it is grabbed. --> + {#snippet grip(key)} onNavDragStart(e, key)} ondragend={clearNavDrag} - aria-hidden="true">⠿ + aria-hidden="true" + > {/snippet} {#snippet projectRow(project, sortable)} @@ -1004,19 +1012,15 @@ so nothing moves when it appears. */ .grip { position: absolute; - left: -2px; - width: 12px; - text-align: center; - font-size: 10px; - line-height: 1; - color: var(--ink-faint); + left: 0; + top: 0; + bottom: 0; + width: 14px; cursor: grab; - opacity: 0; - transition: opacity var(--fast) var(--ease); } - .navrow:hover .grip { - opacity: 1; + .grip:active { + cursor: grabbing; } .navrow > :global(a) {