Proposal: New Mediatype - Container Image Encryption

Overview
We would like to propose a new media type for encrypted layers of a container image. This addition would facilitate the ecosystem for encrypted container images. This allows users with stricter trust requirements to be able ensure end-to-end encryption from build to runtime. In addition, it allows users to use a centralized managed repository (i.e. Docker Hub) without any risk of their images being compromised.
Outdated Design Doc (Look at PR for updated):
https://docs.google.com/document/d/146Eaj7_r1B0Q_2KylVHbXhxcuogsnlSbqjwGTORB8iw/edit?usp=sharing
Link to presentations:
Dockercon US 2019: https://www.youtube.com/watch?v=9LyPUy4XYbs&list=PLkA60AVN3hh-XtoZ8zoZir6wnpaVXGUgk&index=28
Kubecon CN 2019: https://www.youtube.com/watch?v=bzHPnlSfM_8
Links:
Details of updated changes can be viewed in the PR.
Implementation of those changes can be viewed in this PR.
Proposal written and discussed by:
Brandon Lum (@lumjjb), Dimitrios Pendarakis, Hani Jamjoom (@jamjoom), James Bottomley (@jejb), Phil Estes (@estesp), Stefan Berger (@stefanberger), Alaa Youssef within IBM.
This is a follow-up from a discussion with @stevvooe , and @dmcgowan at DockerCon.
Goals
In coming up with a proposal, we considered the following:

Application creators should not need to worry about details of encryption. Flows in the DevOps process and the runtime orchestration would be responsible for facilitating encryption.
Re-usability of layer de-duplication among container images
Encryption should be end-to-end, from the build step to when the container needs to be run on the worker machine. (i.e. registry owner should not be able to view the encrypted contents).
Encrypted images created can be for multiple trusted entities, and key management is responsible for managing that trust.
Specification should be easily integrated to generic with and across key management systems.
Secondary experimental goal: Allow more granular control of security, to provide possibility of desegregation and more fine-grained  security controls. (i.e. base OS, middleware, application can be encrypted separately).

Proposed Changes
Details of updated changes can be viewed in the PR.
Implementation of those changes can be viewed in this PR.

In the creation of a container runtime bundle from the encrypted images, the runtime would perform an additional step of performing the decryption based on the information given in the annotations.
Additional Details
We include some of the interesting discussions we have about the proposal.
Encryption Standard
The encryption/decryption will be done will be according to OpenPGP standard as according to RFC4800.

The layer data will consists of Symmetrically Encrypted Data Packet as in RFC4880 Section 5.7
The wrapped keys (org.opencontainers.image.pgp.keys) will be an array of Public-Key Encrypted Session Key Packets as in RFC4880 Section 5.1
Decryption will be done by processing the wrapped key packets followed by the encrypted data packets.

Key Management
The purpose of Key Management is to assist in performing distribution, storage and use of keys. We note that this is important to be able to ensure that container runtimes are able to obtain the keys for decryption, and for encrypted container image creators to pass keys to their kubernetes/docker runtimes. However, we note that Key Management in itself can be seen as a separate component that assists the use of Encrypted Container Images. Therefore, we treat the process of Key Management as separate from the design of the Encrypted Container Images itself.
We note however, that Key Management is important in ensuring technology adoption. We discuss Key Management briefly. We have two models of key management that we consider, they are not exclusive and can probably be used in tandem.
Own Key Management
Key management is handled by the operator of the cloud, and plugins are provided to allow interfacing with existing Key Management Solutions. In this case, the Key Management Solution (i.e. Vault, Azure Vault, IBM KeyProtect, etc.) need to be trusted by the company (run by themselves or by a trusted service).
An example:
Company A has an existing internal Vault service. To build an image, the build machine or developer generates a symmetric key through Vault to perform encryption of the image. This symmetric key is then stored in the Vault service via an Encrypted Container Image Vault plugin. To run the image, the administrator configures it's kubernetes cluster with a Vault token to use the internal vault service via a Encrypted Container Image Vault plugin.
Fully Centralized Untrusted Key Distribution (FCUKD)
Key management is handled by users and container runtimes interacting with an untrusted Key Distribution server (alongside container image registry). Private keys are still managed individually by users but no additional external party's trust is required (if server is compromised, no keys are lost). In addition, it provides a central location for users to manage. The trust model is similar to that of Docker Notary server.
An example:
To build an image, the build machine or developer generates a symmetric key and performs the encryption of the image. The symmetric key is then wrapped with the public key of the entities/receipients that it allows access to and registered with the FCUKD server.
We spin up a new cluster (that operates uses a docker ID of orgcluster). The kubernetes cluster runtime downloads an encrypted image and identifies it does not have the necessary keys to decrypt the image. The runtime sends a request for the key for the layer to the FCUKD server. The approver (image owner or delegate) gets a notification of the request, and verifies, approves and submits a wrapped key to the system. Thereafter,  the requestor (cluster) can download unwrap and use the symmetric key to decrypt the container image. The approval process may be automated through an Access Control Lists set by the key owner.
We note that it is possible to provide the symmetric key to the Fully Centralized Untrusted Key Distribution system and perform auto-approvals, but it is highly discouraged since it weakens the security of the system.

Re-usability of layer de-duplication among container images.

I'm a little bit worried about this goal for multiple reasons.

Deduplication tables act like a form of compression and we know from BEAST that this general construction can result in attacks on most crypto primitives. In addition, this also allows for fingerprinting of encrypted data (something that would not normally be possible -- because ideally you want the same cleartext to have different ciphertext).

This might make it difficult for us to implement content-defined-chunking for files, as well as removing the sequential archive concept from our format (though of course we would probably just re-design it in that case, but it is something to keep in mind).

Encrypted backup tools like restic do have very interesting designs for chunk-deduplicated snapshot-based filesystem images -- have you taken a look at how they work (because that design should influence image-spec if we want to purge tar archives from the format)?

Re-usability of layer de-duplication among container images.

I'm a little bit worried about this goal for multiple reasons.
Deduplication tables act like a form of compression and we know from BEAST that this general construction can result in attacks on most crypto primitives (I'm not sure if this is actually true for dedup tables but it is something to keep in mind).

I don't think this applies to this proposal.  BEAST is based on attacker provided (or attacker-modified) plaintext.  For example, a HTTPS response which contains data from a user-supplied form would be one way for the attacker to influence the plaintext.  But in this case, the encrypter is only signing layers that they've decided to build themselves.  If you're publishing builds of something you develop yourself, there would be no attacker-influenced plaintext.  You'd only run into trouble with BEAST if you were encrypting attacker-influenced layers.  And if you have attacker-influenced layers, you have bigger problems than BEAST ;).

This might make it difficult for us to implement content-defined-chunking for files, as well as removing the sequential archive concept from our format (though of course we would probably just re-design it in that case, but it is something to keep in mind).

This approach (append +enc to the media type and stuff in some annotations) seems pretty generic.  You could even have a Merkle tree where each node was encrypted, although only folks with access to the key would be able to walk that tree for garbage collection and such.  Where the registry-walkable trees are required, you'd have to shuffle things around a bit to encrypt the payloads but not the Merkle links.  So, as you say, potentially some impacts, but nothing that seems difficult to work around.

enc.keyid - The reference to the key to use to perform the decryption of the layer.

Another approach to key distribution would be to encrypt to multiple public keys.  For example, OpenPGP encrypts the payload with a random symmetric key, and then encrypts that symmetric key to one or more public keys.  I don't know how that approach would fit into your enc.algo property, but you could always use additional enc.* properties to support it.  If we want to support that use case, we may want to make enc.keyid an array of strings, instead of making it a single string.

@wking

I don't think this applies to this proposal.

I think you missed the second part of that point, which is that (unrelated to BEAST), taking advantage of deduplication of layers opens you up to fingerprinting attacks (unless the plan is to use a hash of the ciphertext -- in which case you should get zero deduplication because ciphertext should appear random and is not consistent when regenerated).

I think you missed the second part of that point, which is that (unrelated to BEAST), taking advantage of deduplication of layers opens you up to fingerprinting attacks...

Can you link docs for "fingerprinting attacks"?  Searching is turning up things like TOR traffic analysis, which doesn't sound like what you mean.
Going back to the original:

Re-usability of layer de-duplication among container images

For example, if Alice distributes her sensitive application with:

Image 1

Unencrypted base layer(s) (e.g. from an Alpine 3.7 image)
Encrypted app v1.0

Image 2

Different unencrypted base layer(s) (e.g. from an Alpine 3.8 image)
Encrypted app v1.0 (same blob as for image 1)

That's de-duping, because there's only one blob for the encrypted app.  I don't see how it would increase your exposure to attacks, although perhaps the fact that the encrypted app presumably works with both Alpine 3.7 and 3.8 would give you some knowledge of the encrypted content which could be used for a known-plaintext approach.  If you were concerned about that sort of thing, you could always encrypt all of your layers.
And obviously folks will be exposed to ciphertext-only attacks, so you shouldn't be publishing anything that you don't want your grandkids to read ;).
But as long as you're using per-blob session keys (as in the OpenPGP approach I linked above) and periodically rotate your main key (which is only used for signing the short, random session keys), I think this approach seems pretty solid.

@wking

Another approach to key distribution would be to encrypt to multiple public keys. For example, OpenPGP encrypts the payload with a random symmetric key, and then encrypts that symmetric key to one or more public keys. I don't know how that approach would fit into your enc.algo property, but you could always use additional enc.* properties to support it. If we want to support that use case, we may want to make enc.keyid an array of strings, instead of making it a single string.

Do the 'more public keys' in OpenPGP belong to all the people you want to communicate with?
I would say our current thinking is that one would place a encrypted symmetric layer encryption key on some server and decrypt it once an entitled user places a request for the layer decryption  key. The user would have to pass along his public key (certificate) and we would wrap the key with that public key and pass it back. This allows adding users to an access control list for a particular key in the future and pass an individually wrapped key back for each one of them.

Do the 'more public keys' in OpenPGP belong to all the people you want to communicate with?

They could.  You could also encrypt to a key shared by the QA team, and a key shared by the production team, etc.

I would say our current thinking is that one would place a encrypted symmetric layer encryption key on some server and decrypt it once an entitled user places a request for the layer decryption key.

That works too, it's just about whether the encrypted session key is stored on the blob or independently.  If it's on the blob, it's easier to mirror, because it's transmitted through whatever channel you already use for blob mirroring.  If it's in a separate system, you can add/remove recipients without adjusting the blob and rootwards Merkle tree.  I expect that the optimal solution will depend on the individual users and use cases.  And either way we go with this, the other approach is only a new media-type extension away, so recovery is possible if our initial best-guess is wrong.

@stevvooe

I would recommend namespacing the annotation names, per https://github.com/opencontainers/image-spec/blob/master/annotations.md.

Modified the proposal to reflect org.opencontainers.image namespace.

This might need more review from security experts. I'd be worried about defining an algo/keyid schema and would prefer to use something predefined, if possible.

Just to clarify, are you referring to just using a specific algorithm? Or are you looking more for being able to point to an RFC reference of some sort such as https://tools.ietf.org/html/rfc5116 ?

Is there an reference implementation we can use to test this approach with?

Not at the moment. This is our next step. Our current thoughts are to prototype something in containerd for the runtime. We are most definitely open to suggestions on this.
@wking
I think you articulated very clearly what we wanted out of the "de-duplication". Thanks :).

Another approach to key distribution would be to encrypt to multiple public keys. For example, OpenPGP encrypts the payload with a random symmetric key, and then encrypts that symmetric key to one or more public keys. I don't know how that approach would fit into your enc.algo property, but you could always use additional enc.* properties to support it. If we want to support that use case, we may want to make enc.keyid an array of strings, instead of making it a single string.

I really like the idea of wrapping the keys! This would simplify some of the key management problems and infrastructure a lot.
However, I am a little unsure about the scenario where we would want to dynamically allow new users/parties to use our image. A specific case in mind I had was bootstrapping a cluster i.e. when we set up new kubernetes cluster, the cluster would have a CA. And if we wanted to provide access to an encrypted image to a new cluster, we would wrap the symmetric key with the public key of the cluster CA.
I am in favor of using the wrapped keys along with the image as you proposed, and also provide the option to interface with a key management system in the event that the wrapped keys in the registry is not for the user. I am on board for making a wrapped key array.

I suppose the next question is how to implement this and what command line parameters to pass. I suppose docker commit should be instrumented to support this first. docker build may be another later candidate.
Would we want to manage symmetric keys internally somehow with a new set of commands>? We could pass it directly to docker commit --encryption-key file:<path> or reference the key with by name or id if internally managed docker commit --encryption-key name:<keyname>?
Also, how do we pass our one or multiple friends' public key via command line? docker commit --wrapping-pubkey file:<pubkey> and allow multiple of those be passed? Or pass in a config file that references those public keys/certs?
Do we want to give control over the individual layers or just the last one? docker commit --encrypt-layers=<last|all>. And we would refuse to re-encrypt already encrypted layers (in the base image)?

Also, how do we pass our one or multiple friends' public key via command line? docker commit --wrapping-pubkey file:<pubkey> and allow multiple of those be passed?

gpg uses --symmetric <key> for symmetric keys
and --recipient <name|email|key-id?>  (which can be given multiple times) for wrapped session keys.

Do we want to tie this in with gpg in some way or manage recipients in some way ourselves?

The pgp public key server may come in handy...

On Thu, 2018-07-12 at 08:25 -0700, Stefan Berger wrote:

Do we want to tie this in with gpg in some way or manage recipients
in some way ourselves?

Pretty much everything is following the spirit (if not the letter) of
the s/mime spec PKCS#7:
https://tools.ietf.org/html/rfc2315
for Enveloped-data, so this proposal should follow it as closely as is
practicable
James

Pretty much everything is following the spirit (if not the letter) of the s/mime spec PKCS#7: https://tools.ietf.org/html/rfc2315 for Enveloped-data...

That section talks about the same random-session-key-encrypted-to-each-recipient approach.  Do you see a difference between PKCS#7 and OpenPGP on that score?

PGP seems to have its own message format.
Section 5.1 (https://tools.ietf.org/html/rfc4880#section-5) describes the support for multiple recpients:
5.1.  Public-Key Encrypted Session Key Packets (Tag 1)

   A Public-Key Encrypted Session Key packet holds the session key used
   to encrypt a message.  Zero or more Public-Key Encrypted Session Key
   packets and/or Symmetric-Key Encrypted Session Key packets may
   precede a Symmetrically Encrypted Data Packet, which holds an
   encrypted message.  The message is encrypted with the session key,
   and the session key is itself encrypted and stored in the Encrypted
   Session Key packet(s).  The Symmetrically Encrypted Data Packet is
   preceded by one Public-Key Encrypted Session Key packet for each
   OpenPGP key to which the message is encrypted.  The recipient of the
   message finds a session key that is encrypted to their public key,
   decrypts the session key, and then uses the session key to decrypt
   the message.

So this sounds good for supporting multiple recipients if we were to just us OpenPGP tools for creating the encrypted layers. We would follow a standard ... Though, it also ties us into the PGP message format. If we wanted to extend the system with a more dynamic handling of users that can decrypt the layers of an image we'd have to later on be able to parse the OpenPGP message to find the encrypted message and also find an id of the symmetric key.
Ideally we should be able to find the following information somehow either in the OpenPGP data stream or separate the encrypted message and the metadata to decrypt the message in a form like this one here extending the above proposed image annotations:
annotations: {
    enc.keyid: "0x12345678",
    enc.keyid_owner_account: "image-author",
    enc.wrapped : [{
        key_owner<EMAIL_ADDRESS>        key_id: "0x11223344",
        wrapped_key: "0x76923749238749286565...",
    }, {
        key_owner<EMAIL_ADDRESS>        key_id: "0x44332211",
        wrapped_key: "0x983r093275765",
    }]
}

If I am<EMAIL_ADDRESS>I will use my key 0x11223344 to decrypt the wrapped_key part to get to the symmetric key. If I don't find myself in the enc.wrapped key list I can go go the server and ask for enc.key 0x12345678 under account image-author for the symmetric key and will get it back encrypted with my public key assuming I am on the ACL for this key.
Would we want to try an OpenPGP type of decryption of the layer first (assuming the layer is in OpenPGP format) and if this fails fall back to asking the server for the key? I am  just wondering whether OpenPGP is suitable do to this or whether there's some tool implementing PKCS-7 type of messages that seem to be more suitable?

enc.keyid: "0x12345678",
enc.keyid_owner_account: "image-author",

I don't think we need an enc.keyid_owner_account.  Will the symmetric key-store really need to shard these by author?  If you're concerned about garbage collection, I think you want the key-store listening for blob-deletion, so it can remove key 123 when the last blob using that key is deleted.
Similarly, I think we can drop nominal-owner info from the wrapped array.  Owner info will be accessable via the recipient ID (e.g. attached to an OpenPGP key or X.509 cert) where the key<->owner relationship can be signed by others.  I'd be concerned about folks giving unsigned owner assertions here more weight than they deserve.
And if you want, the key-store could have its own public key, and go into the wrapped array too.  Will folks really use the same session key for multiple layers?  Encrypting a random session key to keys.example.com seems safer.  Users not directly authorized (i.e. able to decrypt one of the wrapper payloads) would notice the keys.example.com wrapper and apply for decryption.  As a bonus, this allows one key-store architecture to be "these users are authorized for all blobs", in which case it only needs to store its own key, and not maintain any layer <-> authorized-users mapping.
Also, annotation values must be strings (previous discussion starting here) you'll need to serialize to a string, mint a new descriptor property for the wrapped array, or make an encrytped media type as a separate blob:

manifest's layers[] descriptor points at the enctrypted blob
encrypted has wrapped keys in an array, and a data descriptor pointing at the encrypted layer.
encrypted layer

Would we want to try an OpenPGP type of decryption of the layer first (assuming the layer is in OpenPGP format) and if this fails fall back to asking the server for the key? I am just wondering whether OpenPGP is suitable do to this or whether there's some tool implementing PKCS-7 type of messages that seem to be more suitable?

For both of OpenPGP and S/MIME, the off-the-shelf approach would be to leave the descriptor schema alone and use multipart/encrypted descriptors in the manifest's layers.  Then the referenced blobs would have payloads like this for OpenPGP and this for S/MIME.

@wking The enc.keyid_owner_account would at least reduce the possibility of a key_id collision among different users, though not completely eliminate it (per user) but the key server could refuse two distinct keys (per account) that map to the same key id. It would depend of course how long we make these key IDs for symmetric keys. RFC 4880 does seem to hint at a similar problem for their Key Ids for public keys here. Besides that it's not clear whether their should be a centralized key server that holds much information about keys (and be a high value target) or whether this server forwards requests for symmetric keys to servers that the owners are running themselves. Such request could be forwarded to the owners' server not by key id but by account name.
The above JSON was primarily meant as an example to show what information may be needed.

The enc.keyid_owner_account would at least reduce the possibility of a key_id collision among different users, though not completely eliminate it (per user) but the key server could refuse two distinct keys (per account) that map to the same key id. It would depend of course how long we make these key IDs for symmetric keys.

Yeah.  I think the solution to that is to use longer hashes for the IDs.  I'm not surprised that RFC 4880 is warning about collisions for folks using only 8 bytes ;).  But even with longer IDs, there could still be collisions.  I don't think collisions are a problem though.  If a collision between Alice and Bob's keys makes the target ambiguous, they can each just attempt decryption to see if the payload was really decrypted to them.

Besides that it's not clear whether their should be a centralized key server that holds much information about keys (and be a high value target) or whether this server forwards requests for symmetric keys to servers that the owners are running themselves. Such request could be forwarded to the owners' server not by key id but by account name.

Why not use the key ID as the account name, wherever you're keeping the (key/user)-to-access-server mapping?

@wking Ok, so we can get rid of the account name if the keyid is sufficiently long to be unique and the central server, that would presumably somehow notify the owner of the key, refuses duplicate keyids to be registered with it. [Some sort of registration seems to be necessary.] I suppose for troubleshooting or just being able to contact an owner the central server should be able tell who the key owner is.

To avoid registering bogus key IDs in the central server, one could use the static account info to contact the final server.

@stefanberger, central servers and alternatives seem out of scope here (maybe they would be in-scope for the distribution spec?).  Once you have a set of recipient IDs and payloads encrypted to those IDs, you can have many independent ways of actually decrypting those payloads without impacting the image format.

On Fri, 2018-07-13 at 07:09 -0700, W. Trevor King wrote:

The enc.keyid_owner_account would at least reduce the possibility
of a key_id collision among different users, though not completely
eliminate it (per user) but the key server could refuse two
distinct keys (per account) that map to the same key id. It would
depend of course how long we make these key IDs for symmetric keys.

Yeah.  I think the solution to that is to use longer hashes for the
IDs.  I'm not surprised that RFC 4880 is warning about collisions for
folks using only 8 bytes ;).  But even with longer IDs, there could
still be collisions.  I don't think collisions are a problem
though.  If a collision between Alice and Bob's keys makes the target
ambiguous, they can each just attempt decryption to see if the
payload was really decrypted to them.

I think the proposal has sha256 as the keyid hash, which should not
have collisions (unless it has a serious compromise).

Besides that it's not clear whether their should be a centralized
key server that holds much information about keys (and be a high
value target) or whether this server forwards requests for
symmetric keys to servers that the owners are running themselves.
Such request could be forwarded to the owners' server not by key id
but by account name.

Why not use the key ID as the account name, wherever you're keeping
the (key/user)-to-access-server mapping?

I really think, for an OCI proposal, key distribution should be
feasible but not explicitly spelled out because I can see how you
distribute the keys for the image being a significant cloud native
function, and one that has to comport with all the current keystore
ideas in CNCF, so we want to take an enabling but not prescriptive
approach.
James

... because I can see how you distribute the keys for the image being a significant cloud native function, and one that has to comport with all the current keystore ideas in CNCF, so we want to take an enabling but not prescriptive approach.

Are there CNCF decryption-API proposals?  Then you could authorize decryption at request time "yes, Alice is authorized for key 123 decryptions now, so I'll pass back the decrypted session key".  If instead you distribute a long-running key itself ("Alice is authorized for key 123 now, so I'll pass back its private key"), Alice will have non-revokable access to anything ever encrypted to that key.

If we were to use OpenPGP for managing friends' public keys, then would we also want to use it for the layer encryption directly and take its encrypted output as the encrypted layer? Or only use it to manage public keys? I guess I am not clear what others' opinions are now. I don't think the format is ideal, but I am not sure whether designing our own is better. What I don't like about it is that it encodes Key IDs in the Public-Key Encrypted Session Key Packets that don't give a hint of who these keys are for. If keys are identifiable by their owners' email address, then this information should be preserved I think. The original identifiers of those keys may be useful if some day I were to build a new version of the image or add a new user to it. Those email addresses seem more user friendly than 4 byte key Ids.
The body of this packet consists of:
- A one-octet number giving the version number of the packet type.
       The currently defined value for packet version is 3.

     - An eight-octet number that gives the Key ID of the public key to
       which the session key is encrypted.  If the session key is
       encrypted to a subkey, then the Key ID of this subkey is used
       here instead of the Key ID of the primary key.

     - A one-octet number giving the public-key algorithm used.

     - A string of octets that is the encrypted session key.  This
       string takes up the remainder of the packet, and its contents are
       dependent on the public-key algorithm used.

What I don't like about it is that it encodes Key IDs in the Public-Key Encrypted Session Key Packets that don't give a hint of who these keys are for.

That information is associated with the public key, which you can retrieve by using the key ID.  This is very similar to X.509, where you have a private and public key, as well as a separate certificate asserting the identity of the private-key-holder.  What do you gain by embedding that metadata (as an unsigned assertion) in the recipient list?  If you want to resolve that metadata, you should use the key ID to retrieve metadata which has been signed by parties you trust (e.g. in your web of trust, your trust-on-first-use database, a shared certificate authority, etc.).  For example:
$ gpg --search-keys 0xBB729EC7
gpg: searching for "0xBB729EC7" from hkp server keys.gnupg.net
(1)	CoreOS Application Signing Key<EMAIL_ADDRESS>	  4096 bit RSA key FC8A365E, created: 2016-03-02, expires: 2021-03-01
Keys 1-1 of 1 for "0xBB729EC7".  Enter number(s), N)ext, or Q)uit > q

The analogous PKCS#7 object is similar in just recording the IssuerAndSerialNumber:
RecipientInfo ::= SEQUENCE {
     version Version,
     issuerAndSerialNumber IssuerAndSerialNumber,
     keyEncryptionAlgorithm

       KeyEncryptionAlgorithmIdentifier,
     encryptedKey EncryptedKey }

so you'd have to use that to lookup the cert if you wanted to find the recipient's name.  A difference between the PKCS#7 approach and the OpenPGP approach is that the former only supports one party (the issuer) for asserting metadata (like the recipient's name or domain name), while OpenPGP references the key itself, and allows multiple parties to assert metadata associated with that key.

i like the OpenPGP spec. It seems to be an easy option for users to share keys without a trusted certificate hierarchy. Local users that want to use encrypted containers can use a combination of gpg and docker to run encrypted images (by utilizing the gpg keychain).
In addition, it is convenient that there exist a golang library that implements OpenPGP :) :).
The data layer would be the ciphertext packet and the wrapped keys packets would be the enc.keys array in the annotations.
I will update start updating the original proposal again to note some of the ideas/comments in the discussions.
For the scenario raised where we need to pass keys to new users, @stefanberger and I have discussed  a design with the "Fully Centralized Untrusted Key Server" which will work with the Open PGP model. We have the wrapped keys associated with the image be the trusted parties and have the trusted parties be able to re-wrap the keys.

Updated proposal.

Updated proposal.

Looks like you have stale enc.algo and enc.keyid references now that the meat is all under org.opencontainers.image.enc.keys.
Also, as I mentioned earlier, annotation values must be strings, so the proposal graphic should either move ghe keys property out of annotations or convert the value to a string with something like:
"annotations": {
  "org.opencontainers.image.enc.keys": "[\"wrapping1\",\"wrapping2\"]"
}

And thee seems to be a dangling "registered with a Key ID to the organization namespace" paragraph fragment.  Maybe leftover from a partial edit?

How about new mediaTypes as application/vnd.docker.image.rootfs.diff.tar.gpg and application/vnd.docker.image.rootfs.diff.tar.gzip.gpg?

How about new mediaTypes as application/vnd.docker.image.rootfs.diff.tar.gpg...

GnuPG is just one (popular) implementation.  And + suffixes are a standard media-type approach (although not one we use formally, #332).  How about +pgp forms?

@wking also works

Updated to use mediatype +pgp. How about we change the annotations to s/enc/pgp/g too?

We are making good progress on a POC demo. We're having some troubles with pushing an image - seems to be that mediaType on the top layers (just below SchemaVersion) are not being propagated in the OCI image spec json, and that seems to be causing issue with pushing to a registry.

...seems to be that mediaType on the top layers (just below SchemaVersion) are not being propagated in the OCI image spec json, and that seems to be causing issue with pushing to a registry.

These are just for backwards compat with tools that don't respect descriptor mediaType properties.  If they're lost, it might impact any signatures on them or rootward DAG blobs, but ottherwie shouldn't matter.   What sort of effects are you seeing?

We compared the original image, with our image, and these were the diffs that we saw:
In the top level:
+++ 7a90bfa518320e7c0fe4045776cfc1e7de201ae1d1b6b1322d3c889de3b57ea2    2018-07-27 09:58:14.557632128 -0400
@@ -1,11 +1,10 @@
 {
    "schemaVersion": 2,
-   "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
    "manifests": [
       {
          "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
-         "size": 528,
-         "digest": "sha256:0873c923e00e0fd2ba78041bfb64a105e1ecb7678916d1f7776311e45bf5634b",
+         "digest": "sha256:e86774ce3f8eae7ea551e089e9668c878bdeadd993a4e2f6cdcb497cbb16184a",
+         "size": 456,
          "platform": {
             "architecture": "amd64",
             "os": "linux"```
and in the individual manifests:
```--- 0873c923e00e0fd2ba78041bfb64a105e1ecb7678916d1f7776311e45bf5634b    2018-07-24 16:44:13.673457309 -0400
+++ e86774ce3f8eae7ea551e089e9668c878bdeadd993a4e2f6cdcb497cbb16184a    2018-07-27 09:58:12.900067689 -0400
@@ -1,16 +1,15 @@
 {
    "schemaVersion": 2,
-   "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "config": {```

ERRO[4644] response completed with error err.code="manifest invalid" err.detail="mediaType in manifest should be 'application/vnd.docker.distribution.manifest.v2+json' not ''" err.message="manifest invalid" go.version=go1.10

If the registry requires that property and value in the manifest, I don't see any harm in setting it.  Are you just hunting down some local client code that is stripping it out?

If the registry requires that property and value in the manifest, I don't see any harm in setting it. Are you just hunting down some local client code that is stripping it out?

We are re-creating some of the JSON documents as part of an encryption - decryption cycle. The top level media types are just not written into the documents.
Here's an example where we declare a manifest: https://github.com/stefanberger/containerd/blob/991bedc77ce7dfe2065e48ce22a7b309df431b13/images/image.go#L689
ocispec.Manifest does not have MediaType one could set explicitly and it doesn't seem to get written out (iimplictly) either.
Here's the definition of the Manifest:
https://github.com/stefanberger/containerd/blob/master/vendor/github.com/opencontainers/image-spec/specs-go/v1/manifest.go
That's why the top level MediaType is missing then.

ocispec.Manifest does not have MediaType one could set explicitly...

Because the property is reserved (since #411).  The forward-looking fix would be to update the registry you're using to accept OCI manifests (it looks like it's only currently accepting Docker manifests).  For a short-term fix, you may need to define your own manifest structure which includes the reserved field (like we had before #411) or get one from wherever Docker manifest structures are sold ;).

@wking Can one currently push any images where the JSON documents were (re-)written by containerd? My guess is that our failed attempts to push images should actually be quite common.

Can one currently push any images where the JSON documents were (re-)written by containerd?

I don't know.  The distribution-spec is currently pretty vague about pushing manifests, with two broken links for the types.  But it looks like docker/distribution only landed support for OCI types two weeks ago.  If your registry is using that codebase, maybe that PR has what you need?

CC: @harche who is also contributing.

Versioned struct in docker/distribution has the MediaType declared,
https://github.com/docker/distribution/blob/master/manifest/versioned.go#L6
While in opencontainers/image-spec the Versioned struct doesn't have MediaType,
https://github.com/opencontainers/image-spec/blob/master/specs-go/versioned.go#L20

@harche pointed out an issue with the current build of the registry not branching into the correct routines for the OCI image. He can probably elaborate in much more detail.
We will file an issue with the registry to fix that. In the meantime, we are using a similar mechanism done in buildkit as pointed out by @estesp:
https://github.com/moby/buildkit/blob/master/exporter/containerimage/writer.go#L83-L100
With this, image push/pull works perfectly with the current registry.

We have referenced the current WIP implementation for playing around with! There is a README in the referenced "WIP PR" for the features we wrote.
https://github.com/containerd/containerd/pull/2532
Do let us know if it works well for you - or any issues that come up.

Is there an reference implementation we can use to test this approach with?

@stevvooe here's something to play with :)

@lumjjb Thanks for submitting the PoC! I'll take a closer look at that in detail. I have a few comments in line that should help.

Updated to use mediatype +pgp

Please undo this. ;)

It seems while this proposal was a fairly generic implementation of providing encryption support in descriptors, it seems it has devolved into discussion specific to PGP. Let's focus around the modified mediatype that punts encryption parameters to labels. It is clean, simple, portable and extensible.
PGP is a good test case, but we should also look at other encryption schemes so we can ensure we have an approach that covers other encryption schemes.
When you do encryption, you give up deduplication. @cyphar referenced this with BEAST, but its more than that. We need to acknowledge that this is no longer the goal with encrypted images and pursuing that goal would likely compromise the resulting implementation. Every time you encrypt a layer, there is generally a nonce that should be used that will actually change the cyphertext each time it is encrypted.
It looks like there may be a bug in the registry in requiring the mediaType in the top-level manifests field. That should not be required if using OCI mediatypes.
In general, key distribution is outside the scope of these specifications -- there are a lot of ways to do it. The balance is providing enough information in the layer encryption configuration to allow one to decrypt the image without having to rebuild images when upstream keys/ids/services change.

Hopefully, I am not stomping on the discussion too much and we have enough to move forward.

It seems while this proposal was a fairly generic implementation of providing encryption support in descriptors, it seems it has devolved into discussion specific to PGP. Let's focus around the modified mediatype that punts encryption parameters to labels. It is clean, simple, portable and extensible.

Agreed on the scope. I have some questions about the nature of extensibility though - my worry is that key storage/retrieval needing to be generalized to different encryption schemes - i.e. they may use different key formats and key identifiers. I.e. in OpenPGP we can specify a recipient by email.
There are a few questions that we would need to discuss: 1. "Should key retrieval be specific to encryption scheme?" 2. "If not, How do we handle key retrieval and identification across different encryption schemes?"

PGP is a good test case, but we should also look at other encryption schemes so we can ensure we have an approach that covers other encryption schemes.

We've done a bit of discussion on the topic while working on the PoC, and at least for the usecases that we've thought about, we think that OpenPGP can handle them - and that any variants of supported encryption details would be bindings at the lower level of OpenPGP. I.e. using a HSM routine instead of calling the one in OpenPGP. But definitely, this is a topic that would be nice to hear what others think.

3.When you do encryption, you give up deduplication. @cyphar referenced this with BEAST, but its more than that. We need to acknowledge that this is no longer the goal with encrypted images and pursuing that goal would likely compromise the resulting implementation. Every time you encrypt a layer, there is generally a nonce that should be used that will actually change the cyphertext each time it is encrypted.

Yup - that is true.
We've tried to provide some facilities (i.e. adding recipients without re-encrypting the blob) to help provide some type of deduplication where it would not compromise the security of the content.
The choice of performing encryption on specific layers (that would be supported) allows there to be de-duplication of non-confidential layers (i.e. Building a secret application on top of an ubuntu image, the ubuntu part of the image is not secret.).
Actually, I'm not too familiar with the other types of deduplication that are done. Is there deduplication happening on pages within layers that is happening?

It looks like there may be a bug in the registry in requiring the mediaType in the top-level manifests field. That should not be required if using OCI mediatypes.

Yup - we realized that we had mistakenly specified the docker index type instead, so the registry was going down the wrong codepath. This issue has been resolved appropriately now! :)

In general, key distribution is outside the scope of these specifications -- there are a lot of ways to do it. The balance is providing enough information in the layer encryption configuration to allow one to decrypt the image without having to rebuild images when upstream keys/ids/services change.

Agreed. I think a good candidate interface to key distribution systems are key stores - since most of them work around getting the keys into a particular key store. Given that, we would probably look to implementing an interface for using/handling keys that would follow the generic key model of PKI ( i.e. UnwrapKey,  GetPublicKey, etc. ). This should add pluggability with both services like Vault and more low level interfaces like TPMs and HSMs (which technically could be used through a key store).

@lumjjb

Actually, I'm not too familiar with the other types of deduplication that are done. Is there deduplication happening on pages within layers that is happening?

No, currently the deduplication is only on the layer level. I have a proposal I'm working on (and giving a talk on next week) that will allow for content-defined-chunking to be used for the entire image tree (similar in concept to https://restic.net/). This would then provide deduplication at a much finer level (and give us an opportunity to fix all of the awful tar code).

We've done a bit of discussion on the topic while working on the PoC, and at least for the usecases that we've thought about, we think that OpenPGP can handle them

The main issue with OpenPGP is that the message format is not followed by all implementations. GPG does a lot of things that nothing else does, and many implementations that support the RFC have taken liberties with some of the important undefined behaviour. My impression of OpenPGP (aside from the fact that it still doesn't authenticated encryption -- which should be a strong argument against its use) is that we should not be using it unless we actually want to interact with systems that already auto-consume stuff that has OpenPGP messages in it (such as emailing images).
But that's just my $0.02. I do get why you would use it (and I also felt the same way when working on some of my side projects) but I quickly discovered it has many many many issues.

The main issue with OpenPGP is that the message format is not followed by all implementations. GPG does a lot of things that nothing else does, and many implementations that support the RFC have taken liberties with some of the important undefined behaviour.

Can you cite examples?  Were they with or without the --openpgp compliance option?  Searching for compat issues turned up stuff like this, which doesn't look like a big deal for our use cases.

My impression of OpenPGP (aside from the fact that it still doesn't support authenticated encryption -- which should be a strong argument against its use)...

Really?  The proposed structure here has encryption at the layer/manifest level, with signatures over on the config (I think; there's no opencontainers signature spec, #22, #176, #400, etc.).  And "encrypt the layer and then reference it via a cryptographic hash" (this proposal) is very similar to encrypt-then-MAC, with the difference being hashes vs. HMACs.  Do we care about unforgeability at the encryption/blob level?  I'd expect we only care about it at the signature level.  And if we do care about unforgeability at the blob level, shouldn't we be using an HMACs (alongside raw hashes?) in the Merkle DAG?  I don't think this distinction is something we can narrowly address in an encryption proposal.

... is that we should not be using it unless we actually want to interact with systems that already auto-consume stuff that has OpenPGP messages in it (such as emailing images).

Lots of devs already have OpenPGP keys, and they're a familiar tool in the software-distribution ecosystem.  I think compat with that existing infrastructure is a plus.  S/MIME / PKCS#7 is an extablished alternative in this space, as discussed previously in this issue.  For our purposes, I think the meaningful distinction is whether the ecosystem allows one (S/MIME / PKCS#7) or many (OpenPGP) parties to make assertions about key metadata (see this earlier comment).  Do you have an alternative system you'd like to put on the table?

[...] aside from the fact that it still doesn't support authenticated encryption -- which should be a strong argument against its use) is that we

I don't think authenticated encryption like encrypt-then-MAC can address the problem of a malicious third party replacing a wrapped key and layer. You'd need an image author's signature over the top level manifest (Notary).

Updating the thread with some offline discussions:

the ability for the OCI specification to support the use of other encryption protocols (i.e. the OCI spec should not lock a user into a specific encryption protocol)
Implementing encryption in client vs server side should be explored.
We will investigate into JWK/JWE protocols to see if they are a good fit/option.

We have been looking into JWE/JWK and it seems like a promising approach. It seems to support a rather generic interface. However, at the moment, it does not have support for all protocols, more specifically pgp, tpms, hsms, etc.. We are working with the authors of the RFC standard to try to work in PGP as a key option.
While we try to work towards getting JWE/JWK to be able to support majority of encryption protocol usecases, we have added a field:
org.opencontainers.image.enc.scheme = "pgp"/"jwe"

As support for other protocols in JWE/JWK mature, we will move integration of the protocols into JWE, and hopefully it is possible that it can encompass majority of encryption protocols.

Through the exploration of JWE, we ran into similar issues that we encountered with OpenPGP, but due to the different interfaces exposed, we found it difficult to adopt it the same way we did with OpenPGP. More specifically, it was hard to separate the encrypted data with the rest of the metadata and keys to benefit from deduplication of layer data.
@stevvooe proposed a scheme where we the layer is encrypted by a block cipher and the ciphertext of the encryption scheme used for key delivery be the key of the block cipher.
An rough idea of this would be:
Annotations:
org.opencontainers.image.enc.cipher: "AES_128_GCM"
org.opencontainers.image.enc.cipher.IV: "3r9irw3r3" (or other options)
org.opencontainers.image.enc.key.jwe: "<base64 opengpgp stream>, <base64 opengpgp stream>"

Annotations:
org.opencontainers.image.enc.cipher: "AES_128_GCM"
org.opencontainers.image.enc.cipher.IV: "3r9irw3r3"
org.opencontainers.image.enc.key.jwe: "<base64 JWE json>, <base64 JWE json>"

We don't need so many annotations if we put at least the IV and cipher into the encryptor configuration/metadata which then gets encrypted by OpenPGP or JWE along with the key used for encrypting/decrypting the layer. At least the IV doesn't need to be visible to anyone.
{
    "cipher.iv": "3r9irw3r3" (or other options),
    "cipher": "AES_128_GCM",
    "key": "beg62453abed",
}

It may even save space if we reduce the spaces in the JSON.

I have updated the document to be comment on sign-in (or so I believe so), so that we don't have multiple  "Anonymous" users to facilitate discussion.

Right sorry, I forgot to log in. I was the "Anonymous" user.

Draft KEP: https://docs.google.com/document/d/1NjM58FZFRLtK2qh90WYnk-1nd8ili1d433d8pGwri6g/edit#heading=h.pext71slr0u9

The proposed structure here has encryption at the layer/manifest level, with signatures over on the config (I think; there's no opencontainers signature spec, #22, #176, #400, etc.). And "encrypt the layer and then reference it via a cryptographic hash" (this proposal) is very similar to encrypt-then-MAC, with the difference being hashes vs. HMACs. Do we care about unforgeability at the encryption/blob level?

If you are trying to remove the registry as a trusted party, then you care about authentication. If the registry has control over the manifest and the encrypted blobs, then you need to pick a cipher that provides security under chosen cipher text attacks.

I don't think authenticated encryption like encrypt-then-MAC can address the problem of a malicious third party replacing a wrapped key and layer.

It's less about the ability to replace and more about what you can do with the ability to replace. E.g. someone in control of the registry can use clients as a padding errors if they can detect information about padding errors. With some knowledge of the plaintext, someone in control of the registry can selectively flip bits of the plain text.

Hey @mikedanese, hope you found this interesting! Btw, this issue is a bit stale, we've opened a new PR that you may want to check out - which may be clearer in articulating some of the newer changes: https://github.com/opencontainers/image-spec/pull/775
Yup. The authentication that we suggest for encryption is targeted just for the encrypted blob. Integrity of the image still relies of a separate process of image signing and verification (i.e. with Docker Content Trust and the notary/TUF). We leave those integrity attacks to signing :).

It is not quite the same, as the encryption is also authenticated, we are not just relying on the hash as a MAC. There are some situations in which the registry might guess the plain text (eg if you encrypt a known public base image like Debian, it is not hard to guess what it is from the length), and it is definitely worth doing a full security analysis of the design.

@jterry75 @jstarks - have you reviewed this to make sure it can also work with Windows images?

Tracking implementations:
containerd
https://github.com/containerd/cri/blob/master/docs/decryption.md
https://github.com/containerd/imgcrypt
crio
https://github.com/cri-o/cri-o/blob/master/tutorials/decryption.md
buildah
https://github.com/containers/buildah/pull/2271
skopeo
https://github.com/containers/skopeo/pull/732


Detect callables invoked via register_shutdown_function

Neat, thank you 😃

thanks for the fast merge. do you think we could get this into a 1.x release, so it will not get a PHPStan 2.x only thing ?


Support nested Windows namespace

Fixes #892
Adds a simple test class to ensure this works.

The test case covers the type name appearing in isolation, but I was expecting to also see issues with parameter and field types. Your test class also needs to have methods/properties that use things from a Windows namespace. Windows.Foundation.Uri comes to mind.
At least that's the approach I was planning as I started looking at this.


Add more detailed token consumption output to lavague-tests

In order to do more detailed benchmarks and comparison, having more details than just the total number of consumed tokens would be helpful.
Ideally a breakdown by input/output tokens with the type of llms, show the name of the models used as well.
success rate (%)
input | output
llm
mm_llm
embedding

Maybe we should also add time taken to this print out as well - I think it could be useful @paulpalmieri


[BUG]OpenAIAsyncClient.getChatCompletions.subscribe do not get anything

This code on the graph is not giving me any output. My key is fine, and I can retrieve the data correctly using a non-async method. thx
There is also no output for 'aaaaaa

Hi @wzglgo
Azure SDK for Java (including OpenAI SDK) uses project-reactor as the async non-blocking foundation.
For your sample, to print out the string aaaaaaaa, you can either block it or add a timeout.
See sample

Yes, I tested it with this example from the beginning, but still there is no output at all, the callback function is not being triggered @mssfang


Fix workspace item isn't highlighted when navigating with arrow key

Details

The workspace item isn't highlighted when we use arrow key to navigate.
Fixed Issues

$ https://github.com/Expensify/App/issues/35727
PROPOSAL: https://github.com/Expensify/App/issues/35727#issuecomment-1925181805
Tests

Same as QA Steps

[x] Verify that no errors appear in the JS console

Offline tests

Same as QA Steps
QA Steps

Open workspace switcher page
Make sure Expensify is selected
Use up/down arrow key to navigate the list
Verify the workspace item is highlighted as you navigate through the list.

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

https://github.com/Expensify/App/assets/50919443/e1ece792-fd79-4aba-ad69-c6b07027f651

Android: mWeb Chrome

https://github.com/Expensify/App/assets/50919443/f76e3bd8-5060-415f-a055-ac6b5af12dd2

iOS: Native

https://github.com/Expensify/App/assets/50919443/84b41d9d-307e-4b70-99d9-379368ceab8f

iOS: mWeb Safari

https://github.com/Expensify/App/assets/50919443/fb2471bd-ccbe-4dcd-95be-a1de1a64f866

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/50919443/7938674c-c4ba-41fe-95f5-8799dc69a35a

MacOS: Desktop

https://github.com/Expensify/App/assets/50919443/9e814b2c-0d56-406c-bce5-03ba7195f9f8

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome
https://github.com/Expensify/App/assets/2590174/1978b1f3-f7f5-4505-ab7e-0cf83fabfecb

iOS: Native

iOS: mWeb Safari
https://github.com/Expensify/App/assets/2590174/0e98823e-06e5-46d8-b3ef-bba9ffefbc3c

MacOS: Chrome / Safari
https://github.com/Expensify/App/assets/2590174/f477c27d-14fc-478f-8223-fcd09825a767

MacOS: Desktop
https://github.com/Expensify/App/assets/2590174/39b7db5d-8a5b-461e-8a67-38d1d72e40aa

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to staging by https://github.com/iwiznia in version: 1.4.44-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/puneetlath in version: 1.4.44-13 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


How to connect with existing Next.JS project?

Hi,
I want to know how we can use this with an existing deployed project.
https://github.com/transitive-bullshit/nextjs-notion-starter-kit
Using that repo and want to setup my course using Notion. So, need to have payment gateway activated and membership/auth which seems this repo does well.
Any recommendations on how to do it is appreciated.
Also, how can we add gumroad as well instead of stripe?

Using that repo and want to setup my course using Notion. So, need to have payment gateway activated and membership/auth which seems this repo does well.
Any recommendations on how to do it is appreciated.

You can just pull out the code that you need.

Also, how can we add gumroad as well instead of stripe?

Feel free to open a PR for it, or open an issue and see if someone is willing to add it.


Hypre comm package not cleanly deleted after HypreParMatrix * Add()

The HypreParMatrix returned from HypreParMatrix * Add(double, HypreParMatrix*, double, HypreParMatrix*) does not cleanly delete its CommPkg when using hypre 2.19.0 (and probably other versions).
To reproduce this issue, run the following code through valgrind:
#include "mfem.hpp"
using namespace mfem;

int main(int argc, char *argv[])
{
   MPI_Init(&argc, &argv);
   const char *mesh_file = "../data/star.mesh";
   Mesh mesh(mesh_file, 1, 1);
   int dim = mesh.Dimension();
   ParMesh pmesh(MPI_COMM_WORLD, mesh);

   FiniteElementCollection *fec = new H1_FECollection(1, dim);
   ParFiniteElementSpace fespace(&pmesh, fec);

   ParBilinearForm a(&fespace);
   a.AddDomainIntegrator(new DiffusionIntegrator);
   a.Assemble();
   OperatorPtr A;
   Array<int> empty;
   a.FormSystemMatrix(empty, A);

   HypreParMatrix * hA = A.As<HypreParMatrix>();
   {
      HypreParMatrix * hAT = hA->Transpose();
      HypreParMatrix * yet_another = Add(1.0, *hA, 0.5, *hAT);
      delete hAT;
      delete yet_another;
   }

   delete fec;
   MPI_Finalize();
   return 0;
}

If the part in inner braces is removed, this runs valgrind clean. The valgrind trace points to the Add function, not the Transpose().

I don't see anything suspicious in the Add function or in hypre_ParCSRMatrixAdd.
Hopefully this is not due to some change in the handling of CommPkg... we have that in quite a few places.
Just to check -- do you see this issue with ParAdd and ParMult?

This may be related to https://github.com/mfem/mfem/issues/1750

Adding bug label to prevent the "stale bot" from marking the issue as stale (I think the todo label does not do that at the moment).

@barker29, were you able to get any additional information on this?

For Add as in the code snippet above, it runs valgrind clean up to and including hypre v2.13.0 and then you see memory leaks in v2.14.0 (I only looked at tagged versions of hypre, didn't try to go finer grained than that). The valgrind details point to the CommPkg as not being deleted.
For ParMult, everything is fine up to and including v2.18.2 and then you get "still reachable" memory in v2.19.0, which is probably not as serious a concern as the "definitely lost" memory in Add. I thought at first this was also the CommPkg but now I think it's something else.

Maybe the issue with ParMult is related to the new HYPRE_Finalize() which I think was added in v2.19.0.
For, Add we can try the valgrind option --track-origins=yes to see where the leaked memory was allocated -- that may give us some clue.

The issue with ParMult is indeed related to HYPRE_Init(), HYPRE_Finalize() (see #1276 , #1750).
The issue with Add, ParAdd is slightly different and is fixed in #2147

Discussion continued in #2147.


Can CMO extension support icache management?

I look CMO spec, and I feel that cmo don't manage icache.
So I am confused.

You are correct.  The CMO spec focuses on management of data caching.  The separate "I/D Consistency" extension that is in the works in the J group specifically focuses on management of instruction caching - especially in the context of self and cross modifying code (such as occurs with JIT'ed languages for example).

thanks for your replay.
I will close this issue.


Comment at the end of the mapping key unexpectedly starts a new line

Using v0.2.0
Input
jobs:
  dprint:  # comment
    runs-on: ubuntu-24.04

Expected
jobs:
  dprint: # comment
    runs-on: ubuntu-24.04

Actual
jobs:
  dprint:
    # comment
    runs-on: ubuntu-24.04

Thank you!


docker-compose up causes the cannot unmarshal number into Go struct field

Describe the bug
docker-compose up causes the following error.
Creating hummingbot ... error

ERROR: for hummingbot  Cannot create container for service hummingbot: invalid JSON: json: cannot unmarshal number into Go struct field LogConfig.HostConfig.LogConfig.Config of type string

ERROR: for hummingbot  Cannot create container for service hummingbot: invalid JSON: json: cannot unmarshal number into Go struct field LogConfig.HostConfig.LogConfig.Config of type string
ERROR: Encountered errors while bringing up the project.

Steps to reproduce

Clone the repo
Checkout development branch.
docker-compose up

Release version
development@63f257c
Type of installation
Source
Attach required files
No response

Fixed with #6594

I got confused.
In the installation guide, for Linux users, recommends building from source if you want to modify the source code. Should I do it that way, or with Docker?

I see. The contribution guideline does not explain how to run locally, that's how I got into this confusion. Now it's cleared. Thanks!


Deny missing debug implementations and missing documentation for public symbols.

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.66%. Comparing base (247a572) to head (9e73983).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
- Coverage   75.71%   75.66%   -0.05%
==========================================
  Files          22       22
  Lines        1688     1689       +1
==========================================
  Hits         1278     1278
- Misses        410      411       +1

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


16bit support: Corrupted dir pair at {0x1, 0x0}: Read error after successful writing

Okay. Where to start.
I'm using littleFS with Winbond W25N01GV Nand flash device. In order to access the flash device read, write and erase functions have been created.
All three functions have been tested without littleFS using both Microchip dsPIC33CH512MP508 and PIC32MZ2048EFH144:

Reading pages
Writing pages
Erasing blocks
…
No issues so far.

After that littleFS is added with the following settings:
const struct lfs_config cfg = {
// block device operations
.read  = flash_read,
.prog  = flash_write,
.erase = flash_erase,
.sync  = NULL,
// block device configuration
.read_size = 512,
.prog_size = 512,
.block_size = 2048UL * 64,
.block_count = 1024 - 1,
.cache_size = 512,
.lookahead_size = 16,
.block_cycles = 1000,
};

prog_size has been chosen to be 512 Bytes because the Flash device allows four partial programming accesses to each single page (2048 Bytes/page)
block_count is 1024 – 1 because I don’t want to use the first block of the Flash device. It contains the so called unique ID page and other information that can only be read. Due to this fact read, write and erase function contain an “offset” of one block.
lookahead_size is 16. Can you confirm that this value is a valid one? I’m not sure whether I chose it correctly (found this description: lookahead_size <= block_count / 8)

In order to verify the functionality of littleFS I’ve adapted the “boot_count” example:

After powerup the filesystem is mounted successfully. If mounting wasn’t possible formatting is executed. This happens only when the Flash device has been erased before.
In an endless loop the following array is read, the content of each member incremented by one and after that the array is written. Only a single file is used.
uint8_t test_array[2100] = { 0 };
If a read error occurs unmount followed by mount is executed.

Using Microchip PIC32MZ2048EFH144 I haven’t observed any issues so far.
But using Microchip dsPIC33CH512MP508 a sporadic error occurs:
After many reading and writing cycles as mentioned above I get a read error. Please have a look at the following debug information:
lfs_file_opencfg(2DCE, 3DFC, "filename_1", 102, 3E4A {.buffer=3328, .attrs=0, .attr_count=0})
lfs_file_opencfg -> 0
lfs_file_write(2DCE, 3DFC, 35B8, 2100)
lfs_file_write -> 2100
lfs_file_close(2DCE, 3DFC)
lfs_file_close -> 0
// writing successful
lfs_file_opencfg(2DCE, 3DF8, "filename_1", 101, 3E46 {.buffer=3328, .attrs=0, .attr_count=0})
lfs_file_opencfg -> 0
lfs_file_read(2DCE, 3DF8, 35B8, 2100)
lfs_file_read -> 2100
lfs_file_close(2DCE, 3DF8)
lfs_file_close -> 0
// reading succesful
lfs_file_opencfg(2DCE, 3DFC, "filename_1", 102, 3E4A {.buffer=3328, .attrs=0, .attr_count=0})
lfs_file_opencfg -> 0
lfs_file_write(2DCE, 3DFC, 35B8, 2100)
lfs_file_write -> 2100
lfs_file_close(2DCE, 3DFC)
lfs_file_close -> 0
// writing successful
lfs_file_opencfg(2DCE, 3DF8, "filename_1", 101, 3E46 {.buffer=3328, .attrs=0, .attr_count=0})
Corrupted dir pair at {0x1, 0x0}
lfs_file_opencfg -> -84
// reading error
lfs_unmount(2DCE)
lfs_unmount -> 0
// unmount successful
lfs_mount(2DCE, 2E3C {.context=2DCE, .read=B85A, .prog=AA68, .erase=E98A, .sync=EA2C, .read_size=512, .prog_size=512, .block_size=131072, .block_count=1023, .block_cycles=1000, .cache_size=512, .lookahead_size=16, .read_buffer=2EF0, .prog_buffer=3100, .lookahead_buffer=3308, .name_max=0, .file_max=0, .attr_max=0})
Corrupted dir pair at {0x1, 0x0}
lfs_mount -> -84
// mount not successful

The data of the used file cannot be accessed at all.
Formatting creates a new file system successfully (including data loss).
Read, write and erase functions also create debug entries if an error occurs. But entries from these functions haven’t been observed.
According to the debug log the last writing was successful. After that the next attempt to read delivers -84 (LFS_ERR_CORRUPT).

So here is my biggest question: Is this error/behavior related to the fact that I’m using a 16 Bit controller?
Many thanks in advance for your hints, explanations and solutions.
Looking forward to hearing from you.

Here’s a short update:
I’ve adjusted my code and added more debug log calls to it including debug information of every single read, write and erase access to my Flash device.
In the following description Block numbers are mentioned. These numbers refer to the LittelFS system.
This is what I observe:

Block 0 and 1 are used in order to store the so called metadata pairs

Actual data is stored to free blocks (Block 0 and 1 are not touched for actual data)

Block 0 and 1 are only erased during compaction

After several hours the following situation occurs:
o	Metadata is updated to Block 0
o	Block 0 is erased and actual data is stored there
o	Metadata is tried to be read from Block 0. This seems not possible. Therefore metadata is extracted from Block 1. This is successful.
Nevertheless some of the history stored in Block 0 is lost.
As a consequence my data is older.
After this hiccup my program continues with periodically reading and writing data as mentioned in my first entry to this issue ( (see above: https://github.com/littlefs-project/littlefs/issues/600#issue-1003984570)

A couple of hours later the strongest hiccup occurs:
o	Metadata is updated to Block 1
o	Block 0 is erased and actual data is stored there
o	Metadata is still updated to Block 1
o	Now Block 1 is erased and actual data is stored there

Now both the latest metadata and the “backup” are lost
After that every access to the filesystem results in -84 (LFS_ERR_CORRUPT)
As a next step I’m trying to check the block allocator.

Hi @geky
I was checking block allocator code:

lfs_alloc_lookahead
lfs_alloc

Both functions contain shift left operations (maximum 31 shifts) using “1U”. For 16bit systems 1U is represented by a 16bit value.
I added a cast and started my test again.
Result: even after 5 days of continuous reading/writing to the file system the error mentioned above did not occur.
Main concerns/questions:

LittleFS 16 Bit support: YES/NO?
(I will continue my integration tests, but) are you aware of further variable declarations/usage/pitfalls similar to the ones included in block allocator?
Do you know other users who have successfully used LittleFS on 16bit machines?

Hi @okosch
I am using similar hardware, Winbond W25N01GV and Microchip dsPIC33EP512MU814.
When I tried to format FS for the first time, I have been getting CRC error.
#594 (trace detail were posted here)
Read/Prog/Erase functions are working properly.
Did you encounter similar issue?
I am really struggling.
Any help would be appreciated.


[REQUEST] Wait for end of reload (Puppeteer)

https://discourse.openbullet.dev/t/puppeteer-wait-for-end-of-reload/2175

https://stackoverflow.com/questions/46948489/puppeteer-wait-page-load-after-form-submit
https://github.com/puppeteer/puppeteer/blob/main/docs/api.md#pagewaitfornavigationoptions


Error: worker._routers is not iterable

View https://github.com/edumeet/edumeet/issues/930
worker._routers is not iterable

Does this code run without errors?
Because I think this misses the logic to handle pipeToRouter code which also uses private variables from the Mediasoup Worker class

No mistake detected (only for me).
I don't understand why this problem appeared. Maybe an evolution of the Mediasoup Worker.

Yeah Mediasoup updated their package to deprecate use of private variables publicly

This is not complete. I will fix this soon.


Titanium 64bit support?

Hi there!
Have you been able to get this running on the Ti 3.5.0 Alpha? By February, all modules need to be updated for 64bit support.
See: http://www.appcelerator.com/blog/2014/11/alpha-version-of-titanium-sdk-64-bit-support-now-available

Hello Iñaki Abete,
I will check it soon and come back to this issue.
Thanks!

I have changed this module to  Compatible with Alpha Version of Titanium SDK 64-bit.


⚠ network has degraded performance

In 504d799, network ($NETWORK_URL) experienced degraded performance:

HTTP code: 400
Response time: 1477 ms

Resolved: network performance has improved in 5fad61c after 18 minutes.


Refactor

@simonbyrne @ivanslapnicar Will either of you have a chance to review this or shall I just go ahead and merge?

Dear Simon, dear David,
there is more to this and I will tak a more thorough look soon.
The situation x.hi=0 and x.lo=a!=0 should not happen in a consistent system.
x.lo should always be the lower part of the mantissa, so in this case the number should be stored as x.hi=a, x.lo=0.
Further, presently I get numbers like x.hi=1, x.lo=1e-60, which should not happen for the same reasons.

Best wishes, Ivan
Prof. dr. sc. Ivan Slapničar
Sveučilište u Splitu / University of Split
FESB
R. Boškovića 32
HR-21000 Split
Hrvatska / Croatia
Phone: + 385 21 305893
Ured/Office: B803

Šalje: David P. Sanders<EMAIL_ADDRESS>Poslano: 21. svibnja 2016. 22:25
Prima: simonbyrne/DoubleDouble.jl
Kopija: Ivan Slapničar; Mention
Predmet: Re: [simonbyrne/DoubleDouble.jl] Refactor (#13)
@simonbyrnehttps://github.com/simonbyrne @ivanslapnicarhttps://github.com/ivanslapnicar Will either of you have a chance to review this or shall I just go ahead and merge?

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com/simonbyrne/DoubleDouble.jl/pull/13#issuecomment-220798532


Why it doesn't have the first step to create the project directory?

It feels, to me, that we are missing the first step. Where the CLI ask the name of the project to create the directory and set that name everywhere.
There a reason why this package skips it?
Do you want help to add it?

Yes this is something i've been meaning to add! I plan on getting to it shortly. I certainly wouldn't object to a PR which adds the functionality.


Pin containers/* dependencies

What type of PR is this?
/kind dependency-change
What this PR does / why we need it:
Pinning the golang deps to their latest release to let dependabot pickup new versions.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?
None

/lgtm


[WIP] Feature/row expand animate

Keep working on #816 to make it configurable
I will working on this and hope to release in next couple sprints
FYI @YassienW

I'll keep my eye on the branch and contribute if i can. Let me know if there is anything i can help with. What exactly would you like to add configuration for?

@YassienW sorry for lately reply, I want to allow user to enable/disable the animation and also the animation way. But I think it's good to me to merge this PR then we can see if any enhancement can provide. I will release this feature in this weekend. thanks


When rebuilt, carousel doesn't work

Describe the bug
When I rebuild the lib (b292ec4849eea5c9c95fe658e35d9a8491434dea) and add it to my website, the slider doesn't move.
To Reproduce
Steps to reproduce the behavior:
yarn run build and add it to your website
Expected behavior
The carousel should move
Screenshots
:woman_shrugging:
Desktop (please complete the following information):
any
Smartphone (please complete the following information):
any
Additional context
When I revert 91f80fa5d2ea11c915bdbadffff50cb7bf859067, it works again.

Hey @orangecms
Can you just install this dependency from npm as described in README? I think that would be easier :)

Sure, I did that, but I ran into #47, so I worked on #74, but I had to rebuild to try it out, and then the carousel stopped moving. So I went back to master, rebuilt, and it still didn't move. I got different build artifacts as well, i.e., git status told me that lib/ had changed, i.e., I couldn't reproduce it. Then I reverted other patches to see where it broke, and that seemed to be the commit I mentioned up there.

@orangecms
Can I ask you to fill in this form https://docs.google.com/forms/d/1tfRs1Io49tJBwz-06aDnz8hrJ3WySJTi45y4k6hHGaQ ?


PHP 8 support

Fixes #111
Checklist

[x] Code follows coding conventions held in this repo
[ ] ~Automated tests have been added~ (no code in src was changed)
[x] Tests are passing
[ ] ~Docs have been updated (if applicable)~ (support is stated as "PHP 7 and above", which covers PHP 8 too)
[x] Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test
API reference can be generated by running vendor/bin/phpdoc

Thank you for PR @jankonas,
I have briefly checked the changes and they look fine. I'll do a proper review next week and also try to validate the version release plan so that we don't need to release multiple major version close to each other.

I have updated https://github.com/Kentico/kontent-delivery-sdk-php/wiki/Doing-PHP-stuff-on-Windows-with-VS-Code accordingly to PHP 8


[SearchField] onKeyDown event handler never called

The documentation for SearchField indicates that it supports an onKeyDown event handler. However, if a handler is registered, it is never called.
I believe the cause is that this code overrides the onKeyDown prop from the spread:
https://github.com/adobe/react-spectrum/blob/562b4006e27f2c479d3498df44c4ea689ff37c3a/packages/%40react-aria/searchfield/src/useSearchField.ts#L87-L93
Expected Behaviour
If I register a handler, it is called.
Actual Behaviour
It isn't.
Reproduce Scenario (including but not limited to)
Add an event handler, generate an event, and observe that the handler isn't called.
Steps to Reproduce

Register a handler
Press a key

Platform and Version
Platform: Mac, but I can't imagine this matters.
Version: 3.8.0, but the problem looks to be still present in main.
Sample Code that illustrates the problem
<SearchField
  aria-label="Search"
  placeholder="Enter text"
  onSubmit={(value: string) => {
    console.log("submitted", value);
  }}
  onClear={() => {
    console.log("cleared");
  }}
  onKeyDown={(event) => {
    console.log("you'll never see me", event);
  }}
  onKeyUp={(event) => {
    console.log("I work correctly", event);
  }}
/>

Logs taken while reproducing problem

Note for the team/whoever picks this up: check if textfield/textarea call the user provided onKeyDown as well.


docs: updated readme with no broken link

Description
Changed the link so it sends you to he official flutterfire_ui explanation and what to use.
Related Issues
Flutterfire github repo's readMe has broken link #10459
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

[x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
[x] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
[x] All existing and new tests are passing.
[x] I updated/added relevant documentation (doc comments with ///).
[x] The analyzer (melos run analyze) does not report any problems on my PR.
[x] I read and followed the Flutter Style Guide.
[x] I signed the CLA.
[x] I am willing to follow-up on review comments in a timely manner.

Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?

[ ] Yes, this is a breaking change.
[x] No, this is not a breaking change.

Closing in favour of: https://github.com/firebase/flutterfire/pull/10509


CAMEL-11111: implemented throwExceptionOnFailure Producer attribute

Thanks for the PR it has been merged. Do you mind closing this?


DSP calculator - can't select "from items of"

Describe the bug
Today I can no longer select anything other than the target item in the "from items/belts/factories of... (items)"
To Reproduce
Steps to reproduce the behavior:

Go to https://factoriolab.github.io/list?p=magnet11&f=1belt-3_assembler-3&s=dsp0***belt-3&v=1
Click on  the magnet to the right of "of"
The magnet icon and the iron ore icon are shown but I cannot select the ore - the same is true for any product, For products with multiple components, nothing but the target item can be selected.

Expected behavior
I should be able to select the target item or any of the components (e.g. produce processors from 1 belt of copper ingot)
Desktop (please complete the following information):

OS: Windows 10 fully updated.
Browser Tested in latest available versions of chrome and edge. Also tested incognito in Chrome.
Version [e.g. 22]

Additional context
It was working as expected on Thursday...

This sounds like the same issue as #492 but in a different dialog. I should be able to apply a similar fix, I'll look into it asap.

Thanks for the superfast turnaround!! All working fine now :)
On Mon, 31 May 2021 at 14:16, Doug Broad @.***> wrote:

Closed #495 https://github.com/factoriolab/factorio-lab/issues/495 via
#496 https://github.com/factoriolab/factorio-lab/pull/496.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/factoriolab/factorio-lab/issues/495#event-4820638979,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABJIBMOFIJSX4RWPGQPUMXLTQOD27ANCNFSM452QO6IQ
.

--
-- Steve


Dialog.Root's onOpenChange callback is not called when dialog open

Discussed in https://github.com/huntabyte/shadcn-svelte/discussions/308

Originally posted by pseriesadmin September 20, 2023
Dialog.Root's onOpenChange callback is not called when dialog open, is this a bug?
<script lang="ts">
  ...

  export let open = false;

  const onOpenChange = (state: boolean | undefined) => {
     console.log(state); // <---- not called when state is true
  };

</script>

<Dialog.Root
  bind:open
  {onOpenChange}
  closeOnEscape={false}
  closeOnOutsideClick={false}
>
...
</Dialog.Root>

@huntabyte Hello, I spent a few hours trying to figure out what I'm doing wrong but found that I'm not alone. It's impossible for me to continue my work because I can't subscribe to the event and do some functions inside it. If we add
$: console.log(open);  to track the state, it works fine, but I can not do some of my functionality inside of it unfortunately as a temporal solution.

@huntabyte Thank you, also form selector component in shadcn is broken with bits-ui 0.9.2 and can't be opened at all. You can check it in docs with form example. https://www.shadcn-svelte.com/docs/components/select#form
Version 0.9.1 works fine for me.

hm I do see that now that you mention in 🤔

Also, onOpenChange is intended to be used for internal changes that are out of your control, and actually the logic of updating your bind:open is handled there. So when you change it outside of it, it would end up calling it twice which I expect you wouldn't want.

Good news, I identified the issue and am working on a patch for it now!

The issue I'm referring to is for the select btw.
As mentioned, the onOpenChange couldn't work the way you expect it to due to how it synchronizes the bind state with the internal state. onOpenChange is designed to be used when you aren't controlling the state from outside the component with bind:open.
Otherwise, it results in behavior like this, where the change function is fired multiple times, once when it changes, and a second time when it syncs the value.
https://github.com/huntabyte/bits-ui/assets/64506580/a804dfcf-6071-4035-99f7-ee5c8b2647cb
I could possibly explore some alternative approaches to this, but is very low on my priority list at the moment.
The idea is that if you are modifying the state outside the component, you know when it is going to change and should apply whatever logic you expect to happen on change there for the time being.

@huntabyte The issue with select is confirmed fixed within this minimal repro I was working up to open a separate issue on! 🎉
https://stackblitz.com/edit/github-lsakyy
Genuinely appreciate all you do for the Svelte community!

Going to close this issue now as the behavior is working as initially intended, maybe in the future during a refactor (maybe svelte 5) we will be able to accommodate this, but at the moment the juice isn't worth the squeeze!


Vertically and horizontally center PDF page

A custom implementation for supporting zooming and moving across the document has been added (in commit f7b9ae7ea651cf3fa201d6001d5fe60ad73f399f) to support the changes related to centering the document.
Without the custom implementation, the user won't be able to properly move across the document, specifically the top-left corner (as the user gets restricted to the actual bounds of the page [once they have sufficiently zoomed in]).
We might require to test or review this PR before we merge these changes.

Doing this via #229.


I want to know the version of webrtc in licode? who can tell me? tks

Description

Steps to reproduce the issue:
1.
2.
3.
Describe the results you received:
Describe the results you expected:
Additional information you deem important (e.g. issue happens only occasionally):
Licode commit/release where the issue is happening
Additional environment details (Local, AWS, Docker, etc.):

There's no version of WebRTC per say but as draft for WebRTC 1.0 is being worked. The WebRTC stack consists of many open source libraries meant for performing different tasks and achieving different functionalities specified under WebRTC specification. Not sure if the version of WebRTC is relevant just yet as it's under active development and standardisation will happen over time.


Example build failure

Fails the build CI requirement outlined in:
https://github.com/milesfrain/stm32template#a-note-on-continuous-integration

CI log output:
arm-none-eabi-g++ "/github/workspace/common/src/usb_task.cpp" -mcpu=cortex-m4 -std=gnu++14 -DUSE_HAL_DRIVER -DSTM32F413xx -DUSE_FULL_LL_DRIVER -c -I../Core/Inc -I../USB_DEVICE/App -I../USB_DEVICE/Target -I"/github/workspace/common/inc" -I../custom/inc -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Drivers/STM32F4xx_HAL_Driver/Inc -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Drivers/STM32F4xx_HAL_Driver/Inc/Legacy -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Middlewares/Third_Party/FreeRTOS/Source/include -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Middlewares/Third_Party/FreeRTOS/Source/CMSIS_RTOS -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Middlewares/Third_Party/FreeRTOS/Source/portable/GCC/ARM_CM4F -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Middlewares/ST/STM32_USB_Device_Library/Core/Inc -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Middlewares/ST/STM32_USB_Device_Library/Class/CDC/Inc -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Drivers/CMSIS/Device/ST/STM32F4xx/Include -I/usr/share/stm_repo/STM32Cube_FW_F4_V1.25.1/Drivers/CMSIS/Include -Os -ffunction-sections -fdata-sections -fno-exceptions -fno-rtti -fno-threadsafe-statics -fno-use-cxa-atexit -Wall -fstack-usage -MMD -MP -MF"common/src/usb_task.d" -MT"common/src/usb_task.o" --specs=nano.specs -mfpu=fpv4-sp-d16 -mfloat-abi=hard -mthumb -o "common/src/usb_task.o"
In file included from /github/workspace/common/src/uart_tasks.cpp:11:0:
/github/workspace/common/inc/dma_reg.h: In function 'uint32_t dmaFlagCheckAndClear(DMA_TypeDef*, uint32_t, DmaFlag)':
/github/workspace/common/inc/dma_reg.h:44:10: error: 'struct DMA_TypeDef' has no member named 'HIFCR_OOPS'; did you mean 'HIFCR'?
     dma->HIFCR_OOPS = mask;
          ^~~~~~~~~~
          HIFCR
make: *** [common/src/subdir.mk:46: common/src/uart_tasks.o] Error 1
make: *** Waiting for unfinished jobs....
"make -j2 all" terminated with exit code 2. Build might be incomplete.

22:49:26 Build Failed. 3 errors, 0 warnings. (took 1s.283ms)