Hash history comments and sign them with the author's personal key. by ChrisPenner · Pull Request #6004 · unisonweb/unison

ChrisPenner · 2025-11-14T19:21:55Z

Overview

Adds a migration which hashes and signs all of a codebase's history comments, as a step towards syncing them with Share.

Implementation approach and notes

Adds a Migration which:

Creates a Personal Key for the user.
- A personal key is a randomly generated EdDSA public/private key pair which is stored in the user's credentials.json file.
- This key can be used to assert the user's identity across code-servers in a cryptographically verifiable way.
Maps over all the user's history comments and hashes and signs them, we'll need the hash and author for syncing with share.
Extracts the credentials management into its own package, since we need to use it in the migration and can't depend on unison-cli there.
~~ensure hashing uses a canonicalized serialization of the components to ensure no hash-collisions that can result froma simple concatenation~~

Interesting/controversial decisions

It's hard to know whether personal keys are overkill or not, while it would likely be possible to get away with just Share auth in the short term, I don't think it's really sound in the presence of multiple codeservers; I opted to implement personal keys because I believe I'll get mileage out of them in the future, e.g. they can be used for things like service-account auth in CI and such.

Test coverage

Verify that the credentials.json is ported correctly.

I tested the migration by creating some comments on a trunk build and running the migration on it, then creating some new comments.

Loose ends

Next up is to implement the comment sync APIs.

aryairani · 2025-11-14T20:12:03Z

We need a passphrase option here, right? Or what's the standard

ChrisPenner · 2025-11-14T20:52:28Z

@aryairani

We need a passphrase option here, right? Or what's the standard

Hrmm, this is a deep rabbit hole to go down that adds a ton of work;

Do we support various keychains?
Yubikeys/physical devices?
Various password managers?
gpg agent?

I was planning on having ucm manage these complete for the user as basic EdDSA keys, rather than having the user configure their own keys; thus it's basically treated the same as your Share token, but with the added ability of cryptographic signing and a public key.

Do you think passphrases or the other mentioned features are table-stakes?

ChrisPenner · 2025-11-18T00:25:47Z

codebase2/codebase-sqlite/sql/021-hash-history-comments.sql

Part of the migration, this adds nullable versions of the new columns.

ChrisPenner · 2025-11-18T00:26:46Z

codebase2/codebase-sqlite/sql/022-hash-history-comments-cleanup.sql

Part 3 of the migration, this makes the new columns non-nullable in SQLite's own annoying way, which is creating a new table and copying all the data between them, then dropping the original and renaming the new one.

ChrisPenner · 2025-11-18T00:27:14Z

codebase2/codebase-sqlite/U/Codebase/Sqlite/HistoryComment.hs

This just moved due to import needs.

ChrisPenner · 2025-11-18T00:28:04Z

codebase2/codebase-sqlite/U/Codebase/Sqlite/Queries.hs

Some changes here to support the new comment hashes, author thumbprints, and author signatures.

ChrisPenner · 2025-11-18T00:30:38Z

lib/unison-credentials/src/Unison/Auth/CredentialManager.hs

Now that migrations need a credential manager I rewrote the bulk of the credential manager stuff to support a proper singleton, which should be safer when multiple parts of the app may have their own credential managers; I also pulled it out into its own package so it can be accessed in the Migration.

ChrisPenner · 2025-11-18T00:31:49Z

lib/unison-credentials/src/Unison/Auth/PersonalKey.hs

All new Personal Key stuff.

A personal key is just a public-private EdDsa key pair, which is stored in the user's credentials.json file.

unison-hashing-v2/src/Unison/Hashing/V2/HistoryComments.hs

ChrisPenner · 2026-01-07T00:02:21Z

unison-cli/src/Unison/Auth/CredentialManager.hs

Moved into the unison-credentials package

ChrisPenner · 2026-01-07T00:03:19Z

unison-cli/package.yaml

    - http-types
    - ki
    - lens
-    - lock-file


Moved into unison-credentials

aryairani · 2026-01-07T23:46:48Z

it's basically treated the same as your Share token, but with the added ability of cryptographic signing and a public key.

Okay, makes sense. Sorry I just spotted your reply now.

Edit: It was a draft PR at the time so I feel less bad about not noticing the comment.

aryairani · 2026-01-07T23:50:29Z

Hrmm, this is a deep rabbit hole to go down that adds a ton of work;

Do we support various keychains?

Yubikeys/physical devices?

Various password managers?

gpg agent?

Which approach does command-line ssh use?

lib/unison-sqlite/unison-sqlite.cabal

aryairani

after reverting the cabal hpack version lines (sorry) I would say it can be merged

ChrisPenner · 2026-01-08T20:10:44Z

@aryairani

Hrmm, this is a deep rabbit hole to go down that adds a ton of work;

Do we support various keychains?

Yubikeys/physical devices?

Various password managers?

gpg agent?

Which approach does command-line ssh use?

ssh does support encrypted keys of course, in SSH's case that key represents your ability to sign into remote servers,
in our case, currently the key is only used to signify the author of a comment.

If we're legitimately concerned about a user's credentials file being stolen or leaked then we shouldn't be storing unencrypted auth tokens on disk at all; since currently an attacker would just take the user's OAuth token rather than their personal key.

AFAIK storing unencrypted credentials is quite a common practice, e.g. the AWS cli just stores your unencrypted creds at ~/.aws/credentials; google does the same in ~/.config/gcloud/

Not saying it's bad to allow requiring a password for this stuff, and maybe if we end up using the personal key for accessing Unison Cloud we may wish to add more layers of security, but at the moment I don't think it's worth the time to implement, since if an attacker has access to your file-system we have bigger concerns than them fraudulently signing comments in your name;

Hrmm, Is that a reasonable stance?

aryairani · 2026-01-08T20:19:01Z

Makes sense.
🤞 for CI

sellout · 2026-01-12T20:51:52Z

The Cabal smoke test failure seems like a simple one – need to add the new unison-credentials package to contrib/cabal.project.

The Weeder failures seem to be due to CI using Weeder 2.7.0 vs scripts/check.sh using Weeder 2.8.0 (see freckle/weeder-action/issues/146). I replicated it by changing the version of Weeder used locally, and got the same weeds.

It doesn’t make sense to downgrade Weeder locally, because the weeds that 2.7.0 found aren’t real weeds – just a bug in older Weeder versions.
Upgrading to GHC 9.6.6 with the current Nix infrastructure would be a major hassle for a minor fix.
Replace Nix tooling & update toolchain #6046 will get scripts/check.sh and CI using the same Weeder (2.10.0), but is blocked on some optimization/benchmark work.

I suggest that this PR disable the Weeder check in CI. It‘ll be re-instated with the correct version when #6046 is merged. Independently, we can remove freckle/weeder-action in favor of running scripts/check-weeds in CI to avoid this sync issue in future.

It seems to work now.

ChrisPenner · 2026-01-16T22:50:28Z

It looks like this was merged red, but it was merged as part of a later branch that made it green ✅

ChrisPenner force-pushed the cp/change-comments-sync branch 2 times, most recently from 37b15a0 to aa40862 Compare November 18, 2025 00:09

ChrisPenner commented Nov 18, 2025

View reviewed changes

codebase2/codebase-sqlite/U/Codebase/Sqlite/HistoryComment.hs Outdated

Copy link

Member Author

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just moved due to import needs.

ChrisPenner commented Nov 18, 2025

View reviewed changes

ChrisPenner commented Nov 20, 2025

View reviewed changes

unison-hashing-v2/src/Unison/Hashing/V2/HistoryComments.hs Show resolved Hide resolved

ChrisPenner force-pushed the cp/change-comments-sync branch from 28e1af1 to 0af09bd Compare December 19, 2025 16:18

ChrisPenner force-pushed the cp/change-comments-sync branch 2 times, most recently from 258064e to 6f4d4ac Compare January 6, 2026 18:40

ChrisPenner commented Jan 7, 2026

View reviewed changes

unison-cli/src/Unison/Auth/CredentialManager.hs Outdated

Copy link

Member Author

ChrisPenner Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into the unison-credentials package

ChrisPenner commented Jan 7, 2026

View reviewed changes

unison-cli/package.yaml

- http-types

- ki

- lens

- lock-file

Copy link

Member Author

ChrisPenner Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into unison-credentials

ChrisPenner force-pushed the cp/change-comments-sync branch from abdb970 to 9b4a76e Compare January 7, 2026 00:08

ChrisPenner marked this pull request as ready for review January 7, 2026 00:09

ChrisPenner requested a review from aryairani January 7, 2026 00:12

aryairani reviewed Jan 8, 2026

View reviewed changes

lib/unison-sqlite/unison-sqlite.cabal Outdated Show resolved Hide resolved

aryairani approved these changes Jan 8, 2026

View reviewed changes

ChrisPenner mentioned this pull request Jan 14, 2026

History Comment Sync unisoncomputing/share-api#170

Merged

1 task

ChrisPenner force-pushed the cp/change-comments-sync branch from f4e6821 to 4966a7b Compare January 15, 2026 20:52

ChrisPenner changed the base branch from trunk to topic/array-ops January 15, 2026 21:10

ChrisPenner requested a review from a team as a code owner January 15, 2026 21:10

ChrisPenner and others added 26 commits January 16, 2026 10:56

Migration sql fixes

d18b83b

Add new sql for copying data in order to make non-null columns

a67223d

Fix unqualified sql names

7fd9651

Start wiring in personal keys to migration

52d8ea9

Finish pulling out unison-credentials as a separate package

78ebf02

Retool Credential manager to use a singleton

b7f92fe

Propagate Credential File changes

684870d

Fix up hash migration errors

5aef418

It seems to work now.

Add signature creation and verification for Personal Keys.

716fbd6

Add author signature

814223a

Move history comments hashing module

1391d9b

Wire in PersonalKey Signatures

2889e2c

automatically run ormolu

0f59331

Add in missing author_signature inserts

3e41038

Remove unused blake3 references

f0f788d

Back compat credentials file.

05ede96

Satisfy weeder for now, I'll be using these methods in Share

feab4b5

Add missing hashing dep

978d7ce

Remove debugging

7182ff9

Refactor HistoryComment hashing into Hashing package

ec17839

Normalise comments before hashing using CBOR

6e4e87f

automatically run ormolu

0892db3

Revert cabal hpack versions

730578c

Add unison-credentials to cabal.project

e38f94e

Attempt to satify the weeder gods

0357cd0

automatically run ormolu - and force CI rerun

ef5feec

ChrisPenner force-pushed the cp/change-comments-sync branch from cb0e671 to ef5feec Compare January 16, 2026 18:59

ChrisPenner merged commit ef5feec into trunk Jan 16, 2026
16 of 18 checks passed

ChrisPenner deleted the cp/change-comments-sync branch January 16, 2026 22:49

Conversation

ChrisPenner commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Implementation approach and notes

Interesting/controversial decisions

Test coverage

Loose ends

Uh oh!

aryairani commented Nov 14, 2025

Uh oh!

ChrisPenner commented Nov 14, 2025

Uh oh!

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrisPenner Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisPenner Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

aryairani commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aryairani commented Jan 7, 2026

Uh oh!

Uh oh!

aryairani left a comment

Choose a reason for hiding this comment

Uh oh!

ChrisPenner commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aryairani commented Jan 8, 2026

Uh oh!

sellout commented Jan 12, 2026

Uh oh!

Uh oh!

ChrisPenner commented Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ChrisPenner commented Nov 14, 2025 •

edited

Loading

ChrisPenner Nov 18, 2025 •

edited

Loading

aryairani commented Jan 7, 2026 •

edited

Loading

ChrisPenner commented Jan 8, 2026 •

edited

Loading