Add/remove/replace/info commands added to wbn-sign CLI by rferens · Pull Request #913 · WICG/webpackage

rferens · 2026-03-17T18:31:32Z

Summary
This change refactors the core signing logic into a new high-level SignedWebBundle API, introduces support for multiple signatures

Key Changes

New SignedWebBundle API: Introduced a central class in src/core/signed-web-bundle.ts that provides a simplified, high-level interface for creating, loading, and modifying signed web bundles. It supports:
- Signing an unsigned bundle with one or more strategies.
- Adding or removing signatures from an existing signed bundle.
- Retrieving the Web Bundle ID (App ID).
- Loading signed bundles directly from bytes.
Enhanced Integrity Block Management:
- Moved IntegrityBlock to a dedicated core module and added the ability to parse existing integrity blocks from CBOR.
- Refactored IntegrityBlockSigner to be internal, deprecating several methods in favor of the new SignedWebBundle and utility functions.
Infrastructure & Utilities:
- Added new utility functions for identifying pure vs. signed bundles and verifying signatures.
- Updated tsconfig.json to strip internal declarations from the generated types.

zgroza · 2026-03-20T20:51:53Z

+  /** @internal */
+  async signInternal(): Promise<IntegrityBlock> {


So, essentially this whole class, including the normal .sign() is @internal, isn't it? (I mean, if the constructor is...)

I'd argue that this class should provide a clear interface to the callers: you create it using whatever arguments and then call sign(). But here we have SignedWebBundle calling .signInternal() (which looks like it should only be used from inside this class (and probably should be marked private)), why?

I'd like to have this class internal, but for backward compatiblity(BC) it cannot be at least now (however I think about making it @deprecated now and @internal some day later, when almost no one uses old API.)

Regarding SignedWebBundle using .signInternal - yeah, I though 'internal' for package-internal, not class-internal, but perhaps its just my mistake. as the convention is probably what you suggested. I just created signInternal to be used by SignedWebBundle, which do not do not necessary step like forming Cbor or concatenating to create singeWebBundle.
Again, BC make things harder - the sign method should just return a new integrity block, but because of BC cannot. What do you think about deprecating sign (to change output format one day, when its internal) and renaming this method returning IntegrityBlock (that should be named 'sign')) sth like ' signAndGetIntegrityBlock()'

zgroza · 2026-03-20T21:11:44Z

    return new Uint8Array(buffer);
  }

+  /** @deprecated  Very general function; moved to utils */


Why then leave this here? The whole class has been removed from exports, so removing things from here instead of deprecating will not break anything (more).

The class was not removed from exports. It's kept for backward compatibilty. @deprecated is not a perfect tool for what I want to achieve, but good enough. I want to :
-use some methods still internally
-discourage users from using it externally, but

do not break their current code, but introduce unpleasant warnings

zgroza

This looks good overall, just PTAL at a few last comments.

zgroza · 2026-03-23T14:31:32Z

+      this.pureWebBundle,
+      this.integrityBlock,
+      [signingStrategy]
+    ).signAndGetIntegrityBlock();


I am not really a fan of re-signing every time a new signature is added. Can You sign lazily only when the actual bytes are requested?

We do not resign anything with the same key - creating a signer and signing is still mostly 'additive' and requires mostly O(m) (m-number of new signatures). The only redundant operations, the same every time, are: calculating hash, and formatting stripped IngerityBlock as CBOR and validating it. I believe improving thet to be computed once should be sufficient , is it?

Having SignedWebBundle in the 'lazy state' that does not include new signetures may involve some difficulties and introduce some error-proneness for further implementations. For example when sb. first add singature and later removes same - it would require special implementation for such cases.

I propose then sth else. When Signer finally is depracated we can move out calculating web_bundle hash and cache getting stripped cbor in IntegrityBlock to be calculcated once if not change. WDYT? Would it be ok?

zgroza · 2026-03-23T14:40:11Z

+    this.signingStrategies = signingStrategies;
+  }

+  /** @deprecated This class is only internal, use SignedWebBundle instead. Sign() is going to change interface in next releases. */


Nit: Line length consistency doesn't look in line with the above. Can you verify that everything here passes the same prettier checks?

I think it's how prettier works - It's not maximal length, but just suggested and it sometimes misses long lines. See: this

I'll make sure that those comments are well line-broken anyway.

zgroza · 2026-03-23T14:41:37Z

+    this.signingStrategies = signingStrategies;
+  }

+  /** @deprecated This class is only internal, use SignedWebBundle instead. Sign() is going to change interface in next releases. */


I'm not sure if it's necessary to put those notices on every method if they're already on the constructor.

Okay, I'll replace it with the shorter comment you suggested below: /** @deprecated This class will become only internal in a future release, use SignedWebBundle instead*/

However I'll leave a comment next to @deprecated to explain it anyway

zgroza · 2026-03-23T14:45:44Z

-  }
-
+  // TODO: Move this method to SignedWebBundle/WebBundle class, signer do not need this, especially externally
+  /** @deprecated This class is only internal, use SignedWebBundle instead*/


Like above: it's not internal yet

Yeah, I meant it to be internal for external users but still allowed to used, so marked with @deprecated. But maybe it's not clear enough. Changed.

Apply suggestions from code review Co-authored-by: Zgroza (Luke) Klimek <zgroza@google.com>

This PR expands the CLI with new commands for managing integrity blocks. CLI Improvements: Updated the wbn-sign CLI with a new command-based interface (sign, add-signature, remove-signature, replace-signature, and info), see README for more info. Added colored console output for better readability using the colors package. This is a continuation of #913, which was split into API(original PR) and CLI(this PR) parts. --------- Co-authored-by: Zgroza (Luke) Klimek <zgroza@google.com>

rferens marked this pull request as draft March 17, 2026 18:31

zgroza self-requested a review March 19, 2026 11:22

rferens force-pushed the new_commands branch 5 times, most recently from 39100e9 to a126c8c Compare March 19, 2026 17:32

rferens marked this pull request as ready for review March 19, 2026 17:32

zgroza reviewed Mar 20, 2026

View reviewed changes

rferens force-pushed the new_commands branch from b78a495 to 15e7560 Compare March 21, 2026 23:39

rferens mentioned this pull request Mar 21, 2026

New cli commands #914

Merged

zgroza reviewed Mar 23, 2026

View reviewed changes

rferens force-pushed the new_commands branch from ceb1259 to 30bf533 Compare March 30, 2026 15:37

New API - SignedWebBundle

12b2b5a

Apply suggestions from code review Co-authored-by: Zgroza (Luke) Klimek <zgroza@google.com>

rferens force-pushed the new_commands branch from 30bf533 to 12b2b5a Compare March 30, 2026 15:39

zgroza approved these changes Mar 31, 2026

View reviewed changes

rferens merged commit 19a856d into WICG:main Mar 31, 2026
10 checks passed

		/** @internal */
		async signInternal(): Promise<IntegrityBlock> {

Conversation

rferens commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zgroza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rferens commented Mar 17, 2026 •

edited

Loading