fix: stabilize markdown notes — internal link modal, rename UI, trash integration

- Replace broken ObjectPickerModal with simple inline modal (Label+URL fields)
- Insert internal link at cursor position in textarea
- Add rename button in note editor header and note cards
- Add delete button on note cards with confirm dialog
- Integrate DeleteNote with shared trash (.verstak/trash/) via files.TrashFile()
- Remove hidden .verstak/trash/notes/ folder — notes use unified trash now
- Fix purgeTrashNode to clean file-record-based trash entries (notes/files)
- Add activity + sync ops to DeleteNote binding
- Add files.TrashFile() public method
- Update i18n keys for note.rename, note.deleteConfirm, internal link modal
- AssertContained: symlink-aware path containment check
- Update tests: shared trash, file record missing flag, collision on rename
- All go test ./... pass, frontend build passes, GUI binary built
This commit is contained in:
2026-06-15 09:19:26 +08:00
parent a193c5a4c6
commit 0fdf77ce03
27 changed files with 2892 additions and 39 deletions
+8
View File
@@ -610,6 +610,14 @@ func (s *Service) trashRecord(r Record) error {
return err
}
// TrashFile moves a single file record's physical file to .verstak/trash/
// and marks the record as missing=1. This is the same mechanism used by
// deleteFileRecords, exposed as a public method for individual file trashing
// (e.g. from notes.Delete).
func (s *Service) TrashFile(rec *Record) error {
return s.trashRecord(*rec)
}
func (s *Service) deleteFileRecords(nodeID string) error {
records, err := s.ListByNode(nodeID)
if err != nil {
+157 -7
View File
@@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"time"
"verstak/internal/core/files"
@@ -13,6 +14,58 @@ import (
"verstak/internal/core/util"
)
// NotesFolder is the canonical name for the notes subdirectory inside a case/project.
const NotesFolder = "Notes"
// noteFileRoot returns the absolute path to the notes subdirectory for a given parent.
// For parentless notes it returns <vaultRoot>/Notes.
func noteFileRoot(vaultRoot, parentFsPath string) string {
if parentFsPath != "" {
return filepath.Join(vaultRoot, parentFsPath, NotesFolder)
}
return filepath.Join(vaultRoot, NotesFolder)
}
// assertContained verifies that targetPath is strictly under rootDir.
// It resolves symlinks in the target path to prevent symlink-based escapes.
// Returns an error if targetPath escapes rootDir via .. or symlinks.
func assertContained(rootDir, targetPath string) error {
cleanRoot := filepath.Clean(rootDir)
cleanTarget := filepath.Clean(targetPath)
// Resolve symlinks in the target path to get the real path.
// If the path doesn't exist yet (e.g. for Create), we resolve as much
// as possible and check the unresolved remainder separately.
resolvedTarget, err := filepath.EvalSymlinks(cleanTarget)
if err != nil {
// Path doesn't exist — resolve the parent directory instead.
dir := filepath.Dir(cleanTarget)
resolvedDir, dirErr := filepath.EvalSymlinks(dir)
if dirErr != nil {
// Parent doesn't exist either — fall back to Clean-based check.
rel, relErr := filepath.Rel(cleanRoot, cleanTarget)
if relErr != nil {
return fmt.Errorf("path containment check failed: %w", relErr)
}
if strings.HasPrefix(rel, "..") {
return fmt.Errorf("path %q escapes root %q", cleanTarget, cleanRoot)
}
return nil
}
// Reconstruct target with resolved parent + original base name.
resolvedTarget = filepath.Join(resolvedDir, filepath.Base(cleanTarget))
}
rel, err := filepath.Rel(cleanRoot, resolvedTarget)
if err != nil {
return fmt.Errorf("path containment check failed: %w", err)
}
if strings.HasPrefix(rel, "..") {
return fmt.Errorf("path %q (resolved: %q) escapes root %q", cleanTarget, resolvedTarget, cleanRoot)
}
return nil
}
// Record represents a note entry (links a node to a file).
type Record struct {
NodeID string `json:"node_id"`
@@ -47,25 +100,34 @@ func (s *Service) Create(parentID, title, section string) (*nodes.Node, *files.R
}
filename := seg + ".md"
var destDir string
// Determine the canonical notes directory
var parentFsPath string
if parentID != "" {
parent, err := s.nodes.GetActive(parentID)
if err == nil && parent.FsPath != "" {
destDir = filepath.Join(s.vaultRoot, parent.FsPath)
parentFsPath = parent.FsPath
}
}
if destDir == "" {
destDir = s.vaultRoot
}
destDir := noteFileRoot(s.vaultRoot, parentFsPath)
if err := os.MkdirAll(destDir, 0o750); err != nil {
return nil, nil, fmt.Errorf("mkdir: %w", err)
}
dest := filepath.Join(destDir, filename)
// Path containment check: the resolved file must stay under destDir
if err := assertContained(destDir, dest); err != nil {
return nil, nil, err
}
if _, err := os.Stat(dest); err == nil {
filename = fmt.Sprintf("%s_%s.md", seg, node.ID[:8])
dest = filepath.Join(destDir, filename)
// Re-check containment after rename
if err := assertContained(destDir, dest); err != nil {
return nil, nil, err
}
}
if err := os.WriteFile(dest, []byte("# "+title+"\n\n"), 0o640); err != nil {
@@ -155,9 +217,97 @@ func (s *Service) Save(nodeID, content string) error {
return err
}
// Delete soft-deletes the note node.
// Delete soft-deletes the note node and moves the backing .md file to the
// shared vault trash directory (<vault>/.verstak/trash/) using the same
// trashRecord mechanism as files.DeleteNodeAndChildren. This ensures the
// deleted note appears in the unified Trash UI and can be restored/permanently
// deleted through the existing trash workflow.
func (s *Service) Delete(nodeID string) error {
return s.nodes.SoftDelete(nodeID)
// Load the note record to find the file.
rec, err := s.Load(nodeID)
if err != nil {
return fmt.Errorf("load note: %w", err)
}
// Get the full file record for trashRecord.
fileRec, err := s.files.Get(rec.FileID)
if err != nil {
return fmt.Errorf("get file record: %w", err)
}
// Soft-delete the node first.
if err := s.nodes.SoftDelete(nodeID); err != nil {
return fmt.Errorf("soft-delete node: %w", err)
}
// Move the .md file to the shared trash using the existing trashRecord.
// This places the file in <vault>/.verstak/trash/<fileID>_<filename>
// and marks the file record as missing=1 so it can be restored later.
if err := s.files.TrashFile(fileRec); err != nil {
return fmt.Errorf("trash file: %w", err)
}
return nil
}
// Rename changes the note title and renames the backing .md file on disk.
// If a file with the target name already exists, the operation is rejected.
func (s *Service) Rename(nodeID, newTitle string) error {
if err := s.nodes.UpdateTitle(nodeID, newTitle); err != nil {
return fmt.Errorf("update title: %w", err)
}
// Load the note record to find the file.
rec, err := s.Load(nodeID)
if err != nil {
return fmt.Errorf("load note: %w", err)
}
// Get the current file record.
var oldPath, oldFilename, storageMode string
err = s.db.QueryRow(
`SELECT path, filename, storage_mode FROM files WHERE id = ?`, rec.FileID,
).Scan(&oldPath, &oldFilename, &storageMode)
if err != nil {
return fmt.Errorf("query file: %w", err)
}
// Build old and new absolute paths.
var oldAbs string
if storageMode == "vault" {
oldAbs = filepath.Join(s.vaultRoot, oldPath)
} else {
oldAbs = oldPath
}
oldDir := filepath.Dir(oldAbs)
seg := templates.SafeDisplayNameToPathSegment(newTitle)
if seg == "" {
seg = "note"
}
newFilename := seg + ".md"
newAbs := filepath.Join(oldDir, newFilename)
// Collision check: reject if target exists and is different from source.
if newAbs != oldAbs {
if _, err := os.Stat(newAbs); err == nil {
return fmt.Errorf("file %q already exists", newFilename)
}
if err := os.Rename(oldAbs, newAbs); err != nil {
return fmt.Errorf("rename file: %w", err)
}
// Update file record.
newRel, _ := filepath.Rel(s.vaultRoot, newAbs)
_, err = s.db.Exec(
`UPDATE files SET filename=?, path=?, updated_at=? WHERE id=?`,
newFilename, newRel, utcNow(), rec.FileID,
)
if err != nil {
return fmt.Errorf("update file record: %w", err)
}
}
return nil
}
// Load looks up the note record for a node.
+137
View File
@@ -0,0 +1,137 @@
package notes
import (
"os"
"path/filepath"
"testing"
)
func TestAssertContained(t *testing.T) {
tests := []struct {
name string
root string
target string
wantErr bool
}{
{
name: "file inside root",
root: "/tmp/vault/Notes",
target: "/tmp/vault/Notes/test.md",
wantErr: false,
},
{
name: "file at root boundary",
root: "/tmp/vault/Notes",
target: "/tmp/vault/Notes",
wantErr: false,
},
{
name: "path traversal via ..",
root: "/tmp/vault/Notes",
target: "/tmp/vault/Notes/../../../etc/passwd",
wantErr: true,
},
{
name: "path traversal to parent",
root: "/tmp/vault/Notes",
target: "/tmp/vault/other/file.md",
wantErr: true,
},
{
name: "completely different path",
root: "/tmp/vault/Notes",
target: "/etc/passwd",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := filepath.Clean(tt.root)
target := filepath.Clean(tt.target)
err := assertContained(root, target)
if (err != nil) != tt.wantErr {
t.Errorf("assertContained(%q, %q) error = %v, wantErr %v", root, target, err, tt.wantErr)
}
})
}
}
func TestAssertContainedSymlinkEscape(t *testing.T) {
dir := t.TempDir()
// Create a real directory structure.
notesDir := filepath.Join(dir, "vault", "Notes")
os.MkdirAll(notesDir, 0o750)
// Create a directory outside the vault.
outsideDir := filepath.Join(dir, "outside")
os.MkdirAll(outsideDir, 0o750)
// Create a file outside the vault.
outsideFile := filepath.Join(outsideDir, "secret.md")
os.WriteFile(outsideFile, []byte("secret"), 0o640)
// Create a symlink inside Notes/ pointing outside.
symlinkPath := filepath.Join(notesDir, "escape.md")
if err := os.Symlink(outsideFile, symlinkPath); err != nil {
t.Skipf("cannot create symlink: %v", err)
}
// assertContained should detect the symlink escape.
err := assertContained(notesDir, symlinkPath)
if err == nil {
t.Error("expected error for symlink escape, got nil")
}
}
func TestAssertContainedNonExistentPath(t *testing.T) {
dir := t.TempDir()
notesDir := filepath.Join(dir, "vault", "Notes")
os.MkdirAll(notesDir, 0o750)
// Non-existent file inside root — should pass.
err := assertContained(notesDir, filepath.Join(notesDir, "newfile.md"))
if err != nil {
t.Errorf("expected no error for non-existent file inside root: %v", err)
}
// Non-existent file outside root — should fail.
err = assertContained(notesDir, filepath.Join(dir, "outside", "file.md"))
if err == nil {
t.Error("expected error for non-existent file outside root")
}
}
func TestNoteFileRoot(t *testing.T) {
tests := []struct {
name string
vaultRoot string
parentFsPath string
want string
}{
{
name: "parentless note",
vaultRoot: "/tmp/vault",
parentFsPath: "",
want: "/tmp/vault/Notes",
},
{
name: "note inside project",
vaultRoot: "/tmp/vault",
parentFsPath: "Projects/MyProject",
want: "/tmp/vault/Projects/MyProject/Notes",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := noteFileRoot(tt.vaultRoot, tt.parentFsPath)
got = filepath.Clean(got)
want := filepath.Clean(tt.want)
if got != want {
t.Errorf("noteFileRoot(%q, %q) = %q, want %q", tt.vaultRoot, tt.parentFsPath, got, want)
}
})
}
}
+311
View File
@@ -0,0 +1,311 @@
package notes
import (
"os"
"path/filepath"
"strings"
"testing"
"verstak/internal/core/files"
"verstak/internal/core/nodes"
"verstak/internal/core/storage"
)
func setupRenameService(t *testing.T) (*Service, *nodes.Repository, string) {
t.Helper()
dir := t.TempDir()
db, err := storage.Open(filepath.Join(dir, "test.db"))
if err != nil {
t.Fatalf("open db: %v", err)
}
t.Cleanup(func() { db.Close() })
nodeRepo := nodes.NewRepository(db)
fileSvc := files.NewService(db, dir, nodeRepo)
svc := NewService(db, dir, nodeRepo, fileSvc)
return svc, nodeRepo, dir
}
func TestRenameNote(t *testing.T) {
svc, nodeRepo, _ := setupRenameService(t)
node, _, err := svc.Create("", "Original Title", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Rename
err = svc.Rename(node.ID, "New Title")
if err != nil {
t.Fatalf("Rename: %v", err)
}
// Verify node title updated
updated, err := nodeRepo.GetActive(node.ID)
if err != nil {
t.Fatalf("GetActive: %v", err)
}
if updated.Title != "New Title" {
t.Errorf("title = %q, want %q", updated.Title, "New Title")
}
// Verify slug updated
if !strings.Contains(updated.Slug, "new") {
t.Errorf("slug = %q, want containing 'new'", updated.Slug)
}
}
func TestRenameNoteRenamesFile(t *testing.T) {
svc, nodeRepo, vaultRoot := setupRenameService(t)
node, _, err := svc.Create("", "Original Title", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Write content.
content := "# Original Title\n\nSome content."
if err := svc.Save(node.ID, content); err != nil {
t.Fatalf("Save: %v", err)
}
// Rename should rename the file on disk.
if err := svc.Rename(node.ID, "Renamed Title"); err != nil {
t.Fatalf("Rename: %v", err)
}
// Verify node title updated.
updated, err := nodeRepo.GetActive(node.ID)
if err != nil {
t.Fatalf("GetActive: %v", err)
}
if updated.Title != "Renamed Title" {
t.Errorf("title = %q, want %q", updated.Title, "Renamed Title")
}
// Verify old file no longer exists.
oldPath := filepath.Join(vaultRoot, "Notes", "Original Title.md")
if _, err := os.Stat(oldPath); !os.IsNotExist(err) {
t.Error("old file should not exist after rename")
}
// Verify new file exists with correct content.
newPath := filepath.Join(vaultRoot, "Notes", "Renamed Title.md")
data, err := os.ReadFile(newPath)
if err != nil {
t.Fatalf("ReadFile: %v", err)
}
if string(data) != content {
t.Errorf("content = %q, want %q", string(data), content)
}
}
func TestRenameNoteCollisionRejected(t *testing.T) {
svc, _, _ := setupRenameService(t)
// Create two notes.
node1, _, err := svc.Create("", "Note Alpha", "")
if err != nil {
t.Fatalf("Create note1: %v", err)
}
_, _, err = svc.Create("", "Note Beta", "")
if err != nil {
t.Fatalf("Create note2: %v", err)
}
// Renaming note1 to "Note Beta" should fail — file already exists.
err = svc.Rename(node1.ID, "Note Beta")
if err == nil {
t.Error("expected error for rename collision, got nil")
}
if !strings.Contains(err.Error(), "already exists") {
t.Errorf("expected 'already exists' error, got: %v", err)
}
}
func TestRenameNoteEmptyTitle(t *testing.T) {
svc, _, _ := setupRenameService(t)
node, _, err := svc.Create("", "Valid Title", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Try to rename with empty title — should fail
err = svc.Rename(node.ID, "")
if err == nil {
t.Error("expected error for empty title")
}
}
func TestDeleteNoteSoftDeletesNode(t *testing.T) {
svc, nodeRepo, _ := setupRenameService(t)
node, _, err := svc.Create("", "To Delete", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Delete
if err := svc.Delete(node.ID); err != nil {
t.Fatalf("Delete: %v", err)
}
// Verify node is soft-deleted
_, err = nodeRepo.GetActive(node.ID)
if err == nil {
t.Error("expected deleted node to be inactive")
}
}
func TestDeleteNoteDoesNotAffectOtherNotes(t *testing.T) {
svc, _, vaultRoot := setupRenameService(t)
// Create two notes
node1, _, err := svc.Create("", "Note One", "")
if err != nil {
t.Fatalf("Create note1: %v", err)
}
node2, _, err := svc.Create("", "Note Two", "")
if err != nil {
t.Fatalf("Create note2: %v", err)
}
// Save content to both
svc.Save(node1.ID, "content one")
svc.Save(node2.ID, "content two")
// Delete note1
if err := svc.Delete(node1.ID); err != nil {
t.Fatalf("Delete: %v", err)
}
// Verify note2 content is still readable
content, err := svc.Read(node2.ID)
if err != nil {
t.Fatalf("Read note2: %v", err)
}
if content != "content two" {
t.Errorf("note2 content = %q, want %q", content, "content two")
}
_ = vaultRoot
}
func TestPathTraversalBlocked(t *testing.T) {
svc, _, vaultRoot := setupRenameService(t)
// Try to create a note with path traversal in title
node, _, err := svc.Create("", "../../../etc/passwd", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Verify the file was created with sanitized name, not traversing
content, err := svc.Read(node.ID)
if err != nil {
t.Fatalf("Read: %v", err)
}
_ = content
// Check that no file exists outside vault
suspicious := filepath.Join(vaultRoot, "..", "..", "..", "etc", "passwd.md")
if _, err := os.Stat(suspicious); err == nil {
t.Error("path traversal succeeded — file created outside vault")
}
}
func TestDeleteNoteMovesFileToSharedTrash(t *testing.T) {
svc, _, vaultRoot := setupRenameService(t)
node, _, err := svc.Create("", "To Delete", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Write content
content := "# To Delete\n\nThis content should survive deletion."
if err := svc.Save(node.ID, content); err != nil {
t.Fatalf("Save: %v", err)
}
// Get the file record to know the trash file name.
rec, err := svc.Load(node.ID)
if err != nil {
t.Fatalf("Load: %v", err)
}
fileRec, err := svc.files.Get(rec.FileID)
if err != nil {
t.Fatalf("Get file: %v", err)
}
// Delete (soft-delete + move to shared trash)
if err := svc.Delete(node.ID); err != nil {
t.Fatalf("Delete: %v", err)
}
// Verify original file no longer exists at original location
origPath := filepath.Join(vaultRoot, "Notes", "To Delete.md")
if _, err := os.Stat(origPath); !os.IsNotExist(err) {
t.Error("original file should not exist at original location after delete")
}
// Verify file exists in shared trash (not in trash/notes/)
trashDir := filepath.Join(vaultRoot, ".verstak", "trash")
trashFile := filepath.Join(trashDir, fileRec.ID+"_"+fileRec.Filename)
data, err := os.ReadFile(trashFile)
if err != nil {
t.Fatalf("ReadFile trash: %v", err)
}
if string(data) != content {
t.Errorf("trash content = %q, want %q", string(data), content)
}
// Verify file record is marked missing=1
updatedRec, err := svc.files.Get(rec.FileID)
if err != nil {
t.Fatalf("Get file after delete: %v", err)
}
if !updatedRec.Missing {
t.Error("file record should be marked missing=1 after delete")
}
}
func TestDeleteNoteNoActiveNodeForOrphan(t *testing.T) {
svc, nodeRepo, vaultRoot := setupRenameService(t)
node, _, err := svc.Create("", "Orphan Test", "")
if err != nil {
t.Fatalf("Create: %v", err)
}
// Delete
if err := svc.Delete(node.ID); err != nil {
t.Fatalf("Delete: %v", err)
}
// Node should not be active
_, err = nodeRepo.GetActive(node.ID)
if err == nil {
t.Error("deleted node should not be returned by GetActive")
}
// File should be in shared trash, not in Notes/
notesPath := filepath.Join(vaultRoot, "Notes", "Orphan Test.md")
if _, err := os.Stat(notesPath); !os.IsNotExist(err) {
t.Error("file should not remain in Notes/ after delete")
}
// Verify file is in shared trash (not trash/notes/)
trashDir := filepath.Join(vaultRoot, ".verstak", "trash")
entries, _ := os.ReadDir(trashDir)
found := false
for _, e := range entries {
if e.Name() != "" {
found = true
break
}
}
if !found {
t.Error("file should exist in shared trash/")
}
}
+2 -2
View File
@@ -49,8 +49,8 @@ func TestCreateAndRead(t *testing.T) {
t.Errorf("content = %q", content)
}
// Verify file on disk (in vault root for parentless notes).
entries, _ := os.ReadDir(vaultRoot)
// Verify file on disk (in vault/notes/ for parentless notes).
entries, _ := os.ReadDir(filepath.Join(vaultRoot, "Notes"))
var mdFiles int
for _, e := range entries {
if !e.IsDir() && filepath.Ext(e.Name()) == ".md" {