Skip to content

bug: notes create drops readPermission when writePermission is also set #106

Description

@scode

NOTE: This is generated by AI.

The following is AI generated. It's a real issue I ran into while using hackmd to solve a problem and I believe it to be correct. However I have basically zero experience with TypeScript so actual code suggestions may be of poor quality but I did basic interrogation of the agent about the solution and whether it's idiomatic in context.

notes create drops readPermission when writePermission is also set

hackmd-cli notes create --readPermission=guest --writePermission=owner
creates a note with owner-only read access. The parser drops
readPermission before the API request. team-notes create has the same bug.

I believe the following diff is the required fix:

diff --git a/src/commands/notes/create.ts b/src/commands/notes/create.ts
--- a/src/commands/notes/create.ts
+++ b/src/commands/notes/create.ts
@@ -46,9 +46,9 @@ export default class CreateCommand extends HackMDCommand {
     editor,
     help: Flags.help({char: 'h'}),
     parentFolderId,
-    readPermission: notePermission,
+    readPermission: notePermission(),
     title: noteTitle,
-    writePermission: notePermission,
+    writePermission: notePermission(),
     ...ux.table.flags(),
   }
 
diff --git a/src/commands/team-notes/create.ts b/src/commands/team-notes/create.ts
--- a/src/commands/team-notes/create.ts
+++ b/src/commands/team-notes/create.ts
@@ -37,10 +37,10 @@ export default class Create extends HackMDCommand {
     editor,
     help: Flags.help({char: 'h'}),
     parentFolderId,
-    readPermission: notePermission,
+    readPermission: notePermission(),
     teamPath,
     title: noteTitle,
-    writePermission: notePermission,
+    writePermission: notePermission(),
     ...ux.table.flags(),
   }
 
diff --git a/src/flags.ts b/src/flags.ts
--- a/src/flags.ts
+++ b/src/flags.ts
@@ -43,7 +43,8 @@ export const folderOrder = Flags.string({
   description: 'folder order JSON, e.g. {"root":["folder-id"]}',
 })
 
-export const notePermission = Flags.string({
+/** Return a distinct definition so one permission flag cannot replace another. */
+export const notePermission = () => Flags.string({
   description: 'set note permission: owner, signed_in, guest',
 })
 
diff --git a/test/note-permission-flags.test.ts b/test/note-permission-flags.test.ts
new file mode 100644
--- /dev/null
+++ b/test/note-permission-flags.test.ts
@@ -0,0 +1,27 @@
+import {Parser} from '@oclif/core'
+import {expect} from 'chai'
+
+import CreateNote from '../src/commands/notes/create'
+import CreateTeamNote from '../src/commands/team-notes/create'
+
+describe('note permission flags', () => {
+  for (const [name, command] of [
+    ['personal notes', CreateNote],
+    ['team notes', CreateTeamNote],
+  ] as const) {
+    it(`preserves different read and write permissions for ${name}`, async () => {
+      const {flags} = await Parser.parse(
+        ['--readPermission=guest', '--writePermission=owner'],
+        {
+          flags: {
+            readPermission: command.flags.readPermission,
+            writePermission: command.flags.writePermission,
+          },
+        },
+      )
+
+      expect(flags.readPermission).to.equal('guest')
+      expect(flags.writePermission).to.equal('owner')
+    })
+  }
+})

Why this happens

notePermission is currently one OptionFlag object. Both readPermission
and writePermission register that same object with Oclif:

readPermission: notePermission,
writePermission: notePermission,

With Oclif 2.8.2, parsing different values through the shared object produces
only the second flag:

{"writePermission":"owner"}

flags.readPermission is therefore undefined. The CLI passes that value to
@hackmd/api, JSON serialization omits it, and HackMD applies its owner-only
default. The API itself is not rejecting guest; sending the same two values
directly through @hackmd/api creates the expected public-read, owner-write
note.

The patch makes notePermission a factory. Each command option receives its
own flag definition, and the same parser input becomes:

{"readPermission":"guest","writePermission":"owner"}

This was tested against the exact v2.5.0 tag at
bea38bd5f5f4bdec51e13a02fa725ece9e02eb18.

Reproduction

The script uses authentication saved by hackmd-cli login. It creates one
disposable note, reads its actual permissions through the API, and deletes the
note in a finally block.

Save this as hackmd-cli-read-permission.mjs:

#!/usr/bin/env node

import { execFileSync } from "node:child_process";
import { readFile } from "node:fs/promises";
import { homedir } from "node:os";
import { join } from "node:path";

const configPath = join(
  process.env.HMD_CLI_CONFIG_DIR || join(homedir(), ".hackmd"),
  "config.json",
);
let config = {};
try {
  config = JSON.parse(await readFile(configPath, "utf8"));
} catch (error) {
  if (error.code !== "ENOENT") throw error;
}

const token = process.env.HMD_API_ACCESS_TOKEN || config.accessToken;
const endpoint =
  process.env.HMD_API_ENDPOINT_URL ||
  config.hackmdAPIEndpointURL ||
  "https://api.hackmd.io/v1";
const cli = process.env.HACKMD_CLI || "hackmd-cli";

if (!token) throw new Error("Run hackmd-cli login before running this repro");

const headers = { Authorization: `Bearer ${token}` };
let noteId;

try {
  const output = execFileSync(
    cli,
    [
      "notes",
      "create",
      "--title=test: readPermission repro",
      "--content=# disposable readPermission repro",
      "--readPermission=guest",
      "--writePermission=owner",
      "--output=json",
    ],
    { encoding: "utf8" },
  );
  const created = JSON.parse(output);
  noteId = (Array.isArray(created) ? created[0] : created).id;

  const response = await fetch(`${endpoint}/notes/${encodeURIComponent(noteId)}`, {
    headers,
  });
  if (!response.ok) {
    throw new Error(`GET /notes/${noteId} failed: ${response.status}`);
  }

  const note = await response.json();
  console.log(`requested readPermission: guest`);
  console.log(`actual readPermission:    ${note.readPermission}`);
  console.log(`requested writePermission: owner`);
  console.log(`actual writePermission:    ${note.writePermission}`);

  if (
    note.readPermission !== "guest" ||
    note.writePermission !== "owner"
  ) {
    throw new Error(
      "BUG REPRODUCED: hackmd-cli created the note with the wrong permissions",
    );
  }
} finally {
  if (noteId) {
    const response = await fetch(
      `${endpoint}/notes/${encodeURIComponent(noteId)}`,
      { method: "DELETE", headers },
    );
    if (!response.ok) {
      console.error(`Cleanup failed for https://hackmd.io/${noteId}`);
    }
  }
}

Before applying the patch

From a checkout of v2.5.0:

pnpm install --frozen-lockfile
pnpm build
hackmd-cli login
HACKMD_CLI="$PWD/bin/run" node /path/to/hackmd-cli-read-permission.mjs

The current release exits nonzero with:

requested readPermission: guest
actual readPermission:    owner
requested writePermission: owner
actual writePermission:    owner
Error: BUG REPRODUCED: hackmd-cli created the note with the wrong permissions

After applying the patch

Save the diff above as /tmp/hackmd-cli-read-permission.patch, then run:

git apply /tmp/hackmd-cli-read-permission.patch
pnpm build
HACKMD_CLI="$PWD/bin/run" node /path/to/hackmd-cli-read-permission.mjs

The patched build exits successfully with:

requested readPermission: guest
actual readPermission:    guest
requested writePermission: owner
actual writePermission:    owner

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions