fix(api): UpdateNote reserves updated_at/supersedes/supersede_reason from the caller's fragment (#137)
Closes #125. Skips updated_at/supersedes/supersede_reason in the caller fragment merge loop so they cannot be set via the fragment, only via the intended entry points. delete_branch_after_merge=true Co-authored-by: Mathias <mthbqv@gmail.com>
This commit was merged in pull request #137.
This commit is contained in:
@@ -110,9 +110,22 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
|
|||||||
|
|
||||||
fm, _ := parseFrontmatter(string(prior))
|
fm, _ := parseFrontmatter(string(prior))
|
||||||
for _, l := range contentFM.lines {
|
for _, l := range contentFM.lines {
|
||||||
if l.key != "" {
|
// updated_at/supersedes/supersede_reason are this function's own
|
||||||
fm.set(l.key, l.value)
|
// output, not something a caller's fragment sets — updated_at and
|
||||||
|
// supersedes are already unconditionally re-set below regardless of
|
||||||
|
// what the loop does with them, but supersede_reason is only
|
||||||
|
// re-set when opts.Reason is non-empty, so without this skip a
|
||||||
|
// caller could set it via its own fragment and have it survive
|
||||||
|
// even when Reason is empty, bypassing the one intended entry
|
||||||
|
// point for that field (hyperguild#125, found in #111's review).
|
||||||
|
// Skipping all three here rather than relying on the unconditional
|
||||||
|
// overwrites below to save the other two keeps the invariant
|
||||||
|
// explicit at the one place it can be silently broken by a future
|
||||||
|
// reordering, not implicit in call order.
|
||||||
|
if l.key == "" || l.key == "updated_at" || l.key == "supersedes" || l.key == "supersede_reason" {
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
fm.set(l.key, l.value)
|
||||||
}
|
}
|
||||||
fm.set("updated_at", time.Now().UTC().Format(time.RFC3339))
|
fm.set("updated_at", time.Now().UTC().Format(time.RFC3339))
|
||||||
fm.set("supersedes", priorHash)
|
fm.set("supersedes", priorHash)
|
||||||
|
|||||||
@@ -231,6 +231,45 @@ func TestUpdateThenReadRoundTripsHash(t *testing.T) {
|
|||||||
assert.Equal(t, hash, readHash, "update content_hash must round-trip through ReadNote")
|
assert.Equal(t, hash, readHash, "update content_hash must round-trip through ReadNote")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestUpdateNoteSupersedeReasonNotSettableViaCallerFragmentWhenReasonOptionEmpty(t *testing.T) {
|
||||||
|
// hyperguild#125, found in #111's review: the merge loop applied every
|
||||||
|
// field from the caller's leading frontmatter fragment, including
|
||||||
|
// supersede_reason -- which is only re-set unconditionally when
|
||||||
|
// opts.Reason is non-empty, so a caller could set it via its own
|
||||||
|
// fragment and have it survive even with Reason left empty, bypassing
|
||||||
|
// the one intended entry point for that field.
|
||||||
|
brainDir := t.TempDir()
|
||||||
|
rel := seedNote(t, brainDir, "wiki/a/facts/n.md", "---\nwing: a\nhall: facts\n---\nold\n")
|
||||||
|
|
||||||
|
_, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{
|
||||||
|
Path: rel,
|
||||||
|
Content: "---\nsupersede_reason: snuck in via the fragment\n---\nnew\n",
|
||||||
|
// Reason deliberately left empty.
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel)))
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.NotContains(t, string(got), "supersede_reason",
|
||||||
|
"supersede_reason must not appear at all when opts.Reason is empty, regardless of what the fragment tried to set")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateNoteSupersedeReasonFromOptionStillWorks(t *testing.T) {
|
||||||
|
// Regression pin: the legitimate entry point (opts.Reason) must be
|
||||||
|
// completely unaffected by #125's fix.
|
||||||
|
brainDir := t.TempDir()
|
||||||
|
rel := seedNote(t, brainDir, "wiki/a/facts/n.md", "---\nwing: a\nhall: facts\n---\nold\n")
|
||||||
|
|
||||||
|
_, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{
|
||||||
|
Path: rel, Content: "new\n", Reason: "facts changed",
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel)))
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Contains(t, string(got), "supersede_reason: facts changed")
|
||||||
|
}
|
||||||
|
|
||||||
func TestContentHashStable(t *testing.T) {
|
func TestContentHashStable(t *testing.T) {
|
||||||
assert.Equal(t, ContentHash([]byte("abc")), ContentHash([]byte("abc")))
|
assert.Equal(t, ContentHash([]byte("abc")), ContentHash([]byte("abc")))
|
||||||
assert.NotEqual(t, ContentHash([]byte("abc")), ContentHash([]byte("abd")))
|
assert.NotEqual(t, ContentHash([]byte("abc")), ContentHash([]byte("abd")))
|
||||||
|
|||||||
Reference in New Issue
Block a user