Skip to content

Do not auto-generate keys - #23

Draft
andreabedini wants to merge 1 commit into
mainfrom
dont-autogenerate-keys
Draft

Do not auto-generate keys#23
andreabedini wants to merge 1 commit into
mainfrom
dont-autogenerate-keys

Conversation

@andreabedini

Copy link
Copy Markdown
Collaborator

This makes not signing the repo the default. If the user passes --sign-with-keys KEYS and the path KEYS does not exist, foliage tells the user to create a set of keys.

@michaelpj michaelpj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll need to update our scripts in cardano-haskell-packages, but that's fine.

Do I understand correctly that this still builds a secure repository, just with no signatures? I wonder if we should instead build a non-secure repository in this case. Of the three cases:

  1. Secure repo with signatures
  2. Secure repo without signatures
  3. Insecure repo
    It seems to me like 1 and 3 are the reasonable ones and 2 is a bit odd. Not sure I totally understand what's going on though.

@andreabedini
andreabedini marked this pull request as draft November 7, 2022 13:25
@andreabedini

andreabedini commented Nov 7, 2022

Copy link
Copy Markdown
Collaborator Author

I just noticed this still emits {"keyids":[],"threshold":1}. Threshold set to 1 might be a problem. I need to check.

@bgamari

bgamari commented Mar 21, 2023

Copy link
Copy Markdown
Contributor

I have merged main into this branch so that we can test it in the head.hackage infrastructure.

@andreabedini

Copy link
Copy Markdown
Collaborator Author

I have merged main into this branch so that we can test it in the head.hackage infrastructure.

Thank you @bgamari!

@andreabedini

Copy link
Copy Markdown
Collaborator Author

@michaelpj should we think about merging this? I think CHaP CI will pick up the change and fail (because package.json will be missing the signature and the whole index will change). This would be ok and give us the chance to change the script.

@michaelpj

Copy link
Copy Markdown
Contributor

Yep, seems fine. If you're happy with the CLI interface I'm happy.

@michaelpj

Copy link
Copy Markdown
Contributor

Might need to change the test fixtures also?

@andreabedini

Copy link
Copy Markdown
Collaborator Author

Summary of why this PR has stalled (for future reference):

What it does: Changes foliage build so that not signing is the default. If --sign-with-keys KEYS is passed and the keys directory doesn't exist, foliage errors out with a clear message instead of silently auto-generating keys.

Three unresolved blockers:

  1. CHaP CI breakage. Merging would cause cardano-haskell-packages CI to fail because the signed package.json files would change structure. The consensus was this is acceptable known churn, but nobody pulled the trigger.

  2. TUF correctness. When building without signatures, the repo still emits {"keyids":[],"threshold":1}. A threshold of 1 with zero keys is technically invalid — it should be threshold:0 for an unsigned repo. This was flagged but never followed up.

  3. Test fixtures need updating to cover the new behaviour (noted by @michaelpj).

@michaelpj approved and was comfortable with the CHaP impact. The main remaining work is fixing the threshold:1 issue and updating the test fixtures — both small tasks.

@andreabedini
andreabedini force-pushed the dont-autogenerate-keys branch from add45ab to 0b5648a Compare May 18, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants