Removing the root cert from the bundle's cert chain?

I was thinking about this some more:
https://github.com/sigstore/protobuf-specs/blob/4dbf10bc287d76f1bfa68c05a78f3f5add5f56fe/protos/sigstore_common.proto#L167-L175
...and I think, as specified, it presents a sharp edge towards Sigstore clients: the naive thing to do would be to shove the entire cert chain into the verifying context (i.e. a thing that looks roughly like an OpenSSL X509_STORE). However, if a client does this, then Sigstore signatures effectively become self-signed: any malicious user can insert a cert chain that includes a root CA that isn't one of the trusted ones, and chain validation will resolve against that CA instead.
Clients still need to handle this their own (they should verify the expected contents of the bundle's cert chain), but as a risk reduction strategy: what does everyone think about specifying the root certificate out of the bundle's cert chain? That would turn this from a silent "gotcha" into a spec violation, one that we could write conformance tests against.

https://github.com/sigstore/cosign/issues/2143#issuecomment-1209463170 - my thoughts on this previously
I view it as a verification implementation error to trust the last certificate in the chain without comparing it to a root provided out of band. Happy to revisit this if folks think otherwise!

Yep -- I agree that it's an implementation error to trust the last certificate in the chain, but I also think we can nudge people away from that implementation error by defining the problem away 🙂
From your comment:

TLS does the same, returning a complete chain.

I might be wrong, but I don't think this is true: here's what I get from the TLS handshake for www.google.com:
CONNECTED(00000005)
depth=2 C = US, O = Google Trust Services LLC, CN = GTS Root R1
verify return:1
depth=1 C = US, O = Google Trust Services LLC, CN = GTS CA 1C3
verify return:1
depth=0 CN = www.google.com
verify return:1
write W BLOCK
---
Certificate chain
 0 s:/CN=www.google.com
   i:/C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
-----BEGIN CERTIFICATE-----
MIIEiDCCA3CgAwIBAgIQdRpHZluxJPIKXzgYCivsdzANBgkqhkiG9w0BAQsFADBG
MQswCQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExM
QzETMBEGA1UEAxMKR1RTIENBIDFDMzAeFw0yMzAzMjgxNjU0NThaFw0yMzA2MjAx
NjU0NTdaMBkxFzAVBgNVBAMTDnd3dy5nb29nbGUuY29tMFkwEwYHKoZIzj0CAQYI
KoZIzj0DAQcDQgAE6kUnCX9wl4HT8yKdCpbrW4tFeoKS9lX53ZKskCFwqQiMDiSq
sS96H8y9QqCeKB3unHGY9Ulj/uVhfE/nDi5iAKOCAmgwggJkMA4GA1UdDwEB/wQE
AwIHgDATBgNVHSUEDDAKBggrBgEFBQcDATAMBgNVHRMBAf8EAjAAMB0GA1UdDgQW
BBR7H/f4Obli7c4lJ89UzABm/2VLFTAfBgNVHSMEGDAWgBSKdH+vhc3ulc09nNDi
RhTzcTUdJzBqBggrBgEFBQcBAQReMFwwJwYIKwYBBQUHMAGGG2h0dHA6Ly9vY3Nw
LnBraS5nb29nL2d0czFjMzAxBggrBgEFBQcwAoYlaHR0cDovL3BraS5nb29nL3Jl
cG8vY2VydHMvZ3RzMWMzLmRlcjAZBgNVHREEEjAQgg53d3cuZ29vZ2xlLmNvbTAh
BgNVHSAEGjAYMAgGBmeBDAECATAMBgorBgEEAdZ5AgUDMDwGA1UdHwQ1MDMwMaAv
oC2GK2h0dHA6Ly9jcmxzLnBraS5nb29nL2d0czFjMy9mVkp4YlYtS3Rtay5jcmww
ggEFBgorBgEEAdZ5AgQCBIH2BIHzAPEAdgDoPtDaPvUGNTLnVyi8iWvJA9PL0RFr
7Otp4Xd9bQa9bgAAAYcpW+C7AAAEAwBHMEUCIQDDaZBvW5LdpaxpZuxj4JhVo2Jz
9y1SCBuXqr74f3UWbgIgaBfwz+cLZ7WZjmBUXSpiRAAVwAc/wc12V+SDxxH+AK0A
dwB6MoxU2LcttiDqOOBSHumEFnAyE4VNO9IrwTpXo1LrUgAAAYcpW+ECAAAEAwBI
MEYCIQChIGffyBpm9AaI3x3vIr+aru09qseHIJVfPNWTuO9u+wIhANfvVjMetsSU
/6h2swcmKYvrekoT1mfH2PUXjJ6/j98gMA0GCSqGSIb3DQEBCwUAA4IBAQC1ZzJW
LcT22gys/bV0qgv7/F5/JVZvELGs3htiWpvUTX5MQG9Jm+w6Bu7yflAGdh2B1iCD
qpOHjlRi8NFk15+Q+9i9KNv0et0HZD3o5FXgb8JUxwJov2VbBOBudoHP44VynoHV
9k37j8YNyLjT1M9NuQTZsGA9ohQSSym5ygASZJNDEiuL4kf/fVx7L/9BcWXY3R8z
Bs8UyaY4gbdGBcz0GPutsJTgqqt3lio/qwqjl5y0ZrXQgy5m+g6Nepkwyg83JW+D
c77wcU9cGsJWai9GAJLu6gLGFia+8dGGsyAbmxSdxYPWO+80tf1V/kt6LMnRIyr2
gf3L6kmpMtgnogff
-----END CERTIFICATE-----
 1 s:/C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
   i:/C=US/O=Google Trust Services LLC/CN=GTS Root R1
-----BEGIN CERTIFICATE-----
MIIFljCCA36gAwIBAgINAgO8U1lrNMcY9QFQZjANBgkqhkiG9w0BAQsFADBHMQsw
CQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExMQzEU
MBIGA1UEAxMLR1RTIFJvb3QgUjEwHhcNMjAwODEzMDAwMDQyWhcNMjcwOTMwMDAw
MDQyWjBGMQswCQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZp
Y2VzIExMQzETMBEGA1UEAxMKR1RTIENBIDFDMzCCASIwDQYJKoZIhvcNAQEBBQAD
ggEPADCCAQoCggEBAPWI3+dijB43+DdCkH9sh9D7ZYIl/ejLa6T/belaI+KZ9hzp
kgOZE3wJCor6QtZeViSqejOEH9Hpabu5dOxXTGZok3c3VVP+ORBNtzS7XyV3NzsX
lOo85Z3VvMO0Q+sup0fvsEQRY9i0QYXdQTBIkxu/t/bgRQIh4JZCF8/ZK2VWNAcm
BA2o/X3KLu/qSHw3TT8An4Pf73WELnlXXPxXbhqW//yMmqaZviXZf5YsBvcRKgKA
gOtjGDxQSYflispfGStZloEAoPtR28p3CwvJlk/vcEnHXG0g/Zm0tOLKLnf9LdwL
tmsTDIwZKxeWmLnwi/agJ7u2441Rj72ux5uxiZ0CAwEAAaOCAYAwggF8MA4GA1Ud
DwEB/wQEAwIBhjAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwEgYDVR0T
AQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUinR/r4XN7pXNPZzQ4kYU83E1HScwHwYD
VR0jBBgwFoAU5K8rJnEaK0gnhS9SZizv8IkTcT4waAYIKwYBBQUHAQEEXDBaMCYG
CCsGAQUFBzABhhpodHRwOi8vb2NzcC5wa2kuZ29vZy9ndHNyMTAwBggrBgEFBQcw
AoYkaHR0cDovL3BraS5nb29nL3JlcG8vY2VydHMvZ3RzcjEuZGVyMDQGA1UdHwQt
MCswKaAnoCWGI2h0dHA6Ly9jcmwucGtpLmdvb2cvZ3RzcjEvZ3RzcjEuY3JsMFcG
A1UdIARQME4wOAYKKwYBBAHWeQIFAzAqMCgGCCsGAQUFBwIBFhxodHRwczovL3Br
aS5nb29nL3JlcG9zaXRvcnkvMAgGBmeBDAECATAIBgZngQwBAgIwDQYJKoZIhvcN
AQELBQADggIBAIl9rCBcDDy+mqhXlRu0rvqrpXJxtDaV/d9AEQNMwkYUuxQkq/BQ
cSLbrcRuf8/xam/IgxvYzolfh2yHuKkMo5uhYpSTld9brmYZCwKWnvy15xBpPnrL
RklfRuFBsdeYTWU0AIAaP0+fbH9JAIFTQaSSIYKCGvGjRFsqUBITTcFTNvNCCK9U
+o53UxtkOCcXCb1YyRt8OS1b887U7ZfbFAO/CVMkH8IMBHmYJvJh8VNS/UKMG2Yr
PxWhu//2m+OBmgEGcYk1KCTd4b3rGS3hSMs9WYNRtHTGnXzGsYZbr8w0xNPM1IER
lQCh9BIiAfq0g3GvjLeMcySsN1PCAJA/Ef5c7TaUEDu9Ka7ixzpiO2xj2YC/WXGs
Yye5TBeg2vZzFb8q3o/zpWwygTMD0IZRcZk0upONXbVRWPeyk+gB9lm+cZv9TSjO
z23HFtz30dZGm6fKa+l3D/2gthsjgx0QGtkJAITgRNOidSOzNIb2ILCkXhAd4FJG
AJ2xDx8hcFH1mt0G/FX0Kw4zd8NLQsLxdxP8c4CU6x+7Nz/OAipmsHMdMqUybDKw
juDEI/9bfU1lcKwrmz3O2+BtjjKAvpafkmO8l7tdufThcV4q5O8DIrGKZTqPwJNl
1IXNDw9bg1kWRxYtnCQ6yICmJhSFm/Y3m6xv+cXDBlHz4n/FsRC6UfTd
-----END CERTIFICATE-----
 2 s:/C=US/O=Google Trust Services LLC/CN=GTS Root R1
   i:/C=BE/O=GlobalSign nv-sa/OU=Root CA/CN=GlobalSign Root CA
-----BEGIN CERTIFICATE-----
MIIFYjCCBEqgAwIBAgIQd70NbNs2+RrqIQ/E8FjTDTANBgkqhkiG9w0BAQsFADBX
MQswCQYDVQQGEwJCRTEZMBcGA1UEChMQR2xvYmFsU2lnbiBudi1zYTEQMA4GA1UE
CxMHUm9vdCBDQTEbMBkGA1UEAxMSR2xvYmFsU2lnbiBSb290IENBMB4XDTIwMDYx
OTAwMDA0MloXDTI4MDEyODAwMDA0MlowRzELMAkGA1UEBhMCVVMxIjAgBgNVBAoT
GUdvb2dsZSBUcnVzdCBTZXJ2aWNlcyBMTEMxFDASBgNVBAMTC0dUUyBSb290IFIx
MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAthECix7joXebO9y/lD63
ladAPKH9gvl9MgaCcfb2jH/76Nu8ai6Xl6OMS/kr9rH5zoQdsfnFl97vufKj6bwS
iV6nqlKr+CMny6SxnGPb15l+8Ape62im9MZaRw1NEDPjTrETo8gYbEvs/AmQ351k
KSUjB6G00j0uYODP0gmHu81I8E3CwnqIiru6z1kZ1q+PsAewnjHxgsHA3y6mbWwZ
DrXYfiYaRQM9sHmklCitD38m5agI/pboPGiUU+6DOogrFZYJsuB6jC511pzrp1Zk
j5ZPaK49l8KEj8C8QMALXL32h7M1bKwYUH+E4EzNktMg6TO8UpmvMrUpsyUqtEj5
cuHKZPfmghCN6J3Cioj6OGaK/GP5Afl4/Xtcd/p2h/rs37EOeZVXtL0m79YB0esW
CruOC7XFxYpVq9Os6pFLKcwZpDIlTirxZUTQAs6qzkm06p98g7BAe+dDq6dso499
iYH6TKX/1Y7DzkvgtdizjkXPdsDtQCv9Uw+wp9U7DbGKogPeMa3Md+pvez7W35Ei
Eua++tgy/BBjFFFy3l3WFpO9KWgz7zpm7AeKJt8T11dleCfeXkkUAKIAf5qoIbap
sZWwpbkNFhHax2xIPEDgfg1azVY80ZcFuctL7TlLnMQ/0lUTbiSw1nH69MG6zO0b
9f6BQdgAmD06yK56mDcYBZUCAwEAAaOCATgwggE0MA4GA1UdDwEB/wQEAwIBhjAP
BgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTkrysmcRorSCeFL1JmLO/wiRNxPjAf
BgNVHSMEGDAWgBRge2YaRQ2XyolQL30EzTSo//z9SzBgBggrBgEFBQcBAQRUMFIw
JQYIKwYBBQUHMAGGGWh0dHA6Ly9vY3NwLnBraS5nb29nL2dzcjEwKQYIKwYBBQUH
MAKGHWh0dHA6Ly9wa2kuZ29vZy9nc3IxL2dzcjEuY3J0MDIGA1UdHwQrMCkwJ6Al
oCOGIWh0dHA6Ly9jcmwucGtpLmdvb2cvZ3NyMS9nc3IxLmNybDA7BgNVHSAENDAy
MAgGBmeBDAECATAIBgZngQwBAgIwDQYLKwYBBAHWeQIFAwIwDQYLKwYBBAHWeQIF
AwMwDQYJKoZIhvcNAQELBQADggEBADSkHrEoo9C0dhemMXoh6dFSPsjbdBZBiLg9
NR3t5P+T4Vxfq7vqfM/b5A3Ri1fyJm9bvhdGaJQ3b2t6yMAYN/olUazsaL+yyEn9
WprKASOshIArAoyZl+tJaox118fessmXn1hIVw41oeQa1v1vg4Fv74zPl6/AhSrw
9U5pCZEt4Wi4wStz6dTZ/CLANx8LZh1J7QJVj2fhMtfTJr9w4z30Z209fOU0iOMy
+qduBmpvvYuR7hZL6Dupszfnw0Skfths18dG9ZKb59UhvmaSGZRVbNQpsg3BZlvi
d0lIKO2d1xozclOzgjXPYovJJIultzkMu34qQb9Sz/yilrbCgj8=
-----END CERTIFICATE-----
---
Server certificate
subject=/CN=www.google.com
issuer=/C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
---
No client certificate CA names sent
Server Temp Key: ECDH, X25519, 253 bits
---
SSL handshake has read 4279 bytes and written 374 bytes
---
New, TLSv1/SSLv3, Cipher is AEAD-CHACHA20-POLY1305-SHA256
Server public key is 256 bit
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.3
    Cipher    : AEAD-CHACHA20-POLY1305-SHA256
    Session-ID:
    Session-ID-ctx:
    Master-Key:
    Start Time:<PHONE_NUMBER>
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
---

(via openssl s_client -connect www.google.com:443 -servername www.google.com -showcerts)
i.e. it sends the leaf, then an intermediate, and then a cross-sign for the intermediate, but no actual root CA. Chrome shows the root CA in its cert viewer, but that's only because of a successful out-of-band chain construction 🙂

I also talked a bit with @reaperhulk about the RFC language referenced in https://github.com/sigstore/cosign/issues/2143#issuecomment-1212075858, and we think that this part:

Because
certificate validation requires that trust anchors be distributed
independently, a certificate that specifies a trust anchor MAY be
omitted from the chain, provided that supported peers are known to
possess any omitted certificates.

Refers specifically to intermediates and intermediate caching, given that the first part refers explicitly to the trust anchors (i.e. root CAs) being distributed independently.

Just for some context. We are aware that shipping the entire cert chain can be seen as problematic. The client spec clarifies how to verify an artifact via the bundle:

Perform certification path validation (RFC 5280 §6) of the returned certificate chain with the pre-distributed Fulcio root certificate(s) as a trust anchor.

But if we feel that there is a risk of creating faulty implementations, we may revisit the bundle format. We decided to not allow any public keys to be stored in the bundle for exactly this reason, it could lead to footguns. As the spec mandates that anchoring trust to an OOB delivered root, it was deemed safe to ship the entire chain.

That's purely coincidental though; it I had written the bundle support from scratch I would likely have skipped that part of the client spec and accidentally shoved everything into the chain building context!

I'm not gonna say this is a solution, as it's not (you can't get high quality or security via only testing). But when we get more traction on the conformance testing, I would envision that we have test-cases for scenarios like this -- bundle with roots that does not exist in the trust root and other negative test cases.
Shipping only the leaf certificate makes the most sense from ease of client implementation, and keeps the bundle smaller. I'm wondering if we know the reasoning for the inital Sigstore design to ship the entire chain? As we did replicate that behaviour when creating the bundle. @haydentherapper @znewman01 maybe have more historical context?

I'm not gonna say this is a solution, as it's not (you can't get high quality or security via only testing). But when we get more traction on the conformance testing, I would envision that we have test-cases for scenarios like this -- bundle with roots that does not exist in the trust root and other negative test cases.

Agreed! This is a another strong argument for us pushing forward on bundle test cases in sigstore-conformance (cc @tetsuo-cpp).
I'd also be curious to understand the context/historical reasons for including anything besides the leaf -- I can imagine in making some BYO PKI setups a little easier, but I'm a little hazy there 🙂

Correct, the context was BYO PKI, for those that only provide their root out of band and view intermediates as just chain builders. FWIW, this has been a little messy in Cosign, because we have logic where we first check the TUF root for the intermediates and if it's not present, then we check the bundle/OCI manifest. Sigstore's TUF root does ship the entire chain, and it would be my preference to mandate this because TUF becomes the revocation mechanism rather than deal with CRLs. However we did get some feedback for Cosign where someone said they wanted to use CRLs - While I'd prefer to not support that, if there's enough interest for BYO PKI, we might do so.
One way we could do it is having a required field for the leaf and an optional field for the intermediate chain. We could also do it is a one-off, either setting just the leaf cert or the chain (leaf + intermediates). For the public deployment of sigstore, we'll set only the leaf cert in the bundle, and rely on the TUF trusted root for providing the chain.
This shouldn't complicate client logic any more than it already is. So it becomes:

Create a root and intermediate pool with the certificates from the TUF root

If the chain is set, add those to the intermediate pool only
Verify leaf cert with root and intermediate pools

Thoughts @woodruffw @kommendorkapten ?

the intermediate (or even just the leaf really, since for the public deployments the intermediates are also shipped via TUF) contains sufficient context for chain building, since it tells us the next step in the chain via its AKI extension.

Not necessarily for BYO PKI cases, unless we mandate that intermediates contain the AKI. They should, and something like RFC5280 mandates it iirc. I'm fine saying that chain building isn't possible without the AKI so it should be mandated.

Thoughts @woodruffw @kommendorkapten ?

Making sure I understand: does this mean that in one scenario the bundle would still include the full chain, including the root, or that the chain would only be the leaf + intermediate(s)? If it's the latter, that sounds great to me!

Not necessarily for BYO PKI cases, unless we mandate that intermediates contain the AKI. They should, and something like RFC5280 mandates it iirc. I'm fine saying that chain building isn't possible without the AKI so it should be mandated.

Yeah, I think it would be nice for us to ratchet down on RFC 5280 here and mandate the AKI. @reaperhulk pointed out to me that AKIs aren't technically necessary for chain building (OpenSSL will also compare DNs, apparently), but we have the opportunity to do everything right from scratch 🙂

Making sure I understand: does this mean that in one scenario the bundle would still include the full chain, including the root, or that the chain would only be the leaf + intermediate(s)? If it's the latter, that sounds great to me!

My preference is if we could make this via a patch update, i.e only update the comment to state:

    // The chain of certificates, with indices 0 to n.
    // The first certificate in the array must be the leaf
    // certificate used for signing. Optional intermediate certificates
    // must be stored as offset 1 to n, and the root certificate
    // MUST not be present.

This would not break the message format, and clients that ignores leaf certificates can just grab the first cert.

Requiring SKI and AKI sounds like a good plan 👍

This would not break the message format, and clients that ignores leaf certificates can just grab the first cert.

Oh, that's a good point -- yeah, preserving the wire format would be nice here.

+1, I'm good with that as a plan (That's effectively what Cosign now does anyways, if the intermediates exist in the trust root)

Just realized that this is reused in the trustroot proto too, the chains will contain a root and optionally contain a leaf (the leaf is included for the TSA, but not for Fulcio obviously). So we might need to rethink this. Maybe just update the comment in https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto#L54?

WDYT about two identical (in terms of wire layout) types, but with different names to distinguish their purposes? The trust root could continue to use X509CertificateChain, while the trust bundle could use X509PartialChain or similar.
(If you think that's too complex or not worth the additional codegen, then I'm good with just updating the comment + adding additional client checks during signing and verification that ensure a root isn't present!)

I would vote on what @haydentherapper proposes, update the comment in the bundle that the root must not be present. This is a non-breaking change.
My first gut feeling was to require a client to abort if the bundle contains a root cert, but that could cause problems if a new client verifies an old bundle, so that would be a breaking change.

Do we then need to instruct clients to inspect all certificates in the bundle and ignore any roots if present? So then the comment becomes "The bundle certificate chain SHOULD contain a leaf and MAY contain a set of intermediates. The bundle SHOULD NOT contain the root, but if present, the client MUST ignore the root for verification purposes. The client MUST confirm if the last certificate in the chain is a root or not, by comparing the certificate's Subject and Issuer fields"
Thoughts?

FWIW, I would prefer to keep the full cert chain in the bundle. The java logic we have now expects a full chain, and it verifies that the chain concludes in a known trust anchor. But maybe the Java logic just worked out conveniently that way and I'm being lazy asking for no changes :shrug:
Removing the chain seems like I now have a more complex lookup for the intermediate/root, and do a reconstruction. Is there enough information in the bundle to make that easy?
We already should be doing a few checks here:

certificate was issued when TUF said the root/intermediate chain was valid
certificate chain terminates in a root/intermediate chain from TUF

I think we can still check that clients are behaving via conformance?

My first gut feeling was to require a client to abort if the bundle contains a root cert, but that could cause problems if a new client verifies an old bundle, so that would be a breaking change.

I think we can have our cake and eat it too: if we set an "epoch" after which clients must not include the root in their bundles, then any previously generated bundle (i.e., based on leaf issuance time) can be given a warning and any subsequently generated bundle can produce a hard error.
(This will also make inclusion in the conformance suite easier, since we'll be able to confirm that verification actually fails with a "dangerous" bundle.)
The other option there (which is also reasonable, IMO) is to specify that the bundle MUST NOT contain a root, but that for legacy purposes clients MUST be prepared to handle bundles that contain roots, and that clients MUST handle that root by explicitly ignoring it during chain building. This would be consistent with what RFC 5280 and similar X.509/PKI RFCs do around existing public certificates that don't conform to specs: they specify that conforming implementations MUST NOT do a thing, but that conforming implementations MUST also be prepared to handle well-known non-conforming situations.

FWIW, I would prefer to keep the full cert chain in the bundle. The java logic we have now expects a full chain, and it verifies that the chain concludes in a known trust anchor. But maybe the Java logic just worked out conveniently that way and I'm being lazy asking for no changes 🤷

Yeah, I think it's purely convenience (and a little dangerous) that it worked out that way 🙂. TLS and other ecosystems that I'm aware of all discourage sending full chains, because "accept the chain and check" is much easier to accidentally mess up than "sort into trusted and untrusted and build the chain."

Removing the chain seems like I now have a more complex lookup for the intermediate/root, and do a reconstruction. Is there enough information in the bundle to make that easy?

Yep! Depending on what you're doing (I know very little about Java cryptography), there are two routes I can think of:

You can either use a "store-style" API for chain-building, which your cryptographic library may (should?) already provide: your trusted certificates (from TUF) go in, followed by your untrusted leaf and intermediates (checked explicitly for not containing any root CAs), and then as the store to perform the chain building. That's what sigstore-python does, with OpenSSL's X509_STORE performing all of the logic internally.

You can do the above manually, by starting with your trusted set (from TUF), building then chain up using each untrusted member's AKIs/SKIs, and verifying each signature. This also requires you to verify pathlen and other CA constraints, for strict compliance. I believe this is what sigstore-js does.

You can either use a "store-style" API for chain-building, which your cryptographic library may (should?) already provide: your trusted certificates (from TUF) go in, followed by your untrusted leaf and intermediates (checked explicitly for not containing any root CAs), and then as the store to perform the chain building. That's what sigstore-python does, with OpenSSL's X509_STORE performing all of the logic internally.

This is true for the sigstore go library too (go has a method for chain building). Sigstore-js implements this)

Sorry, just catching up on this.
Overall I'm a  big +1 on taking the root out of the bundle, very carefully. I think it removes a big footgun. Given that:

We should immediately update the spec/protobuf-descriptions: verifiers MUST handle chains without the root certs, signers SHOULD NOT include the root certs.
Verifiers should immediately start handling no-root-cert chains.
Soon after, clients can start producing no-root-cert chains.

I don't actually think a "flag day" is necessary: in terms of backwards compatibility, I don't see a big deal with continuing to accept chains that have the root in them indefinitely. The footgun that we're looking to avoid is depending on this root and accidentally accepting it even if it doesn't come from a trusted source. IIRC a lot of "store-style" APIs will handle this nicely out of the box—does that make sense, esp. @woodruffw ?

signers SHOULD NOT include the root certs.

so only leafs? or everything but root?

I think @znewman01 proposes makes sense (which is very similar to what @woodruffw proposed). It follows the robust principle, be liberate in what you accept (i.e the root may be present but ignored), and strict in what you produce (no roots in the bundle). As we don't have any mass adoption yet, and the clients I'm aware of would not break if we start to remove the root certificates from the bundle we can roll this out "easily". (sigstore-js would not break, and my understanding is that the the Python client would still continue to work.)
For intermediates, we would need to specify the expectations. There is a desire to support the BYO PKI scenario, where the intermediates may not be available via the OOB method. As this is mostly for the non public-good instance, I think we should support it to make it easier to adopt the bundle.
Unless there is a strong will against, I would think it simplifies that if we always require the intermediates to be present in the bundle.
Thoughts?

Yeah I'm okay with anything as long as we have very strong guidance around expectations.

The split seems to be between verification with the public good instance (where intermediates will always be present in TUF) and private instances. Would it make sense that environment be encoded in the bundle? At the very least, we can mark if we're verifying something from PGI vs elsewhere and have stricter verification logic for PGI.

In terms of the bundle, we expect the inputs to a verification to match the Input message: https://github.com/sigstore/protobuf-specs/blob/b6d25769ec1d565ba45fd121310daf4ab963da9b/protos/sigstore_verification.proto#L119-L136
This includes the environment, encoded in a TrustedRoot:  https://github.com/sigstore/protobuf-specs/blob/b6d25769ec1d565ba45fd121310daf4ab963da9b/protos/sigstore_trustroot.proto#L59-L88
as well as general verification options:
https://github.com/sigstore/protobuf-specs/blob/b6d25769ec1d565ba45fd121310daf4ab963da9b/protos/sigstore_verification.proto#L46-L48
It would make sense to me to make this a property of the TrustedRoot; something like CertificateAuthority.allow_other_intermediates: https://github.com/sigstore/protobuf-specs/blob/b6d25769ec1d565ba45fd121310daf4ab963da9b/protos/sigstore_trustroot.proto#L43-L57

Would it make sense that environment be encoded in the bundle? At the very least, we can mark if we're verifying something from PGI vs elsewhere and have stricter verification logic for PGI.

I'm not sure 🤔 This value is unauthenticated and so could be misused. The verifier would still require to make a decision based on a policy on how to verify, so I think your proposal on adding a parameter to the CertificateAuthority is easier to understand and work with. And we put all the decisions to the verifier where they should be.

I guess my concern is that a syntactically-correct but logically-incorrect Sigstore bundle can exploit this as well: we can't rely on clients not to do something. So then we have to add the check anyway.

Yep, agreed that the check needs to happen anyways. I see this more as a way to introduce additional test cases, with explicit success and failure states -- with a MUST NOT, we can add a conformance test case that produces a failure if a client produces a bundle containing a root, whereas SHOULD NOT would mean that the conformance suite is enforcing a superset of restrictions 🙂

I think this still mostly works with openssl-style APIs: you should add only the root as a trusted element of the store.

I might have misled you here: there are no "trusted" or "untrusted" members of an OpenSSL-style store. All members are considered trusted, since the act of adding a member is considered equivalent to trusting it. That's why the "leaf + intermediates + root" case is potentially dangerous with OpenSSL/LibreSSL/BoringSSL/etc.: a client might think that they're only adding untrusted chain-building components, when what they're really telling the API to do is to trust everything 🙃

That's why the "leaf + intermediates + root" case is potentially dangerous with OpenSSL/LibreSSL/BoringSSL/etc.: a client might think that they're only adding untrusted chain-building components, when what they're really telling the API to do is to trust everything 🙃

What sigstore-js does is that it verifies each certificate in the created chain, that it is included in the trusted store provided before using the chain to do path verification.
Approx algorithm is:

Combine trusted roots and intermediates with the root and intermediates from the bundle.
Deduplicate the new set of certs
Find all paths for the leaf certificate
Filter and keep paths that are anchored to the provided trusted root certs
Use the shortest path from the filtered ones

So I feel like a similar algorithm should be fairly easy to implement with an "OpenSSL-style store".

Yep, agreed that the check needs to happen anyways. I see this more as a way to introduce additional test cases, with explicit success and failure states -- with a MUST NOT, we can add a conformance test case that produces a failure if a client produces a bundle containing a root, whereas SHOULD NOT would mean that the conformance suite is enforcing a superset of restrictions 🙂

ACK, makes sense.

I think this still mostly works with openssl-style APIs: you should add only the root as a trusted element of the store.

I might have misled you here: there are no "trusted" or "untrusted" members of an OpenSSL-style store. All members are considered trusted, since the act of adding a member is considered equivalent to trusting it. That's why the "leaf + intermediates + root" case is potentially dangerous with OpenSSL/LibreSSL/BoringSSL/etc.: a client might think that they're only adding untrusted chain-building components, when what they're really telling the API to do is to trust everything 🙃

No, I think I was just a little unclear. I still think this works, it's just a little awkward.  We should probably add an "implementation note" in the client spec.

Approx algorithm is:

Combine trusted roots and intermediates with the root and intermediates from the bundle.
Deduplicate the new set of certs
Find all paths for the leaf certificate
Filter and keep paths that are anchored to the provided trusted root certs
Use the shortest path from the filtered ones

So I feel like a similar algorithm should be fairly easy to implement with an "OpenSSL-style store".

If I'm reading you correctly, this means that a CA could use an intermediate that's not from the trust root, which I think is the whole point of embedded intermediates in the trust root. This point could definitely use clarification in the spec. We can also consider adding "implementation notes" or so.

and so we should not accept unknowns intermediates in the bundle.

This sounds like another good SHOULD restriction: except for the BYO PKI case, bundle should really only ever contain the leaf cert (since everything else will be supplied by the trust root).
(I believe sigstore-python doesn't use any intermediates from the bundle currently, so we shouldn't have any incorrect potential chains here.)

Taking a step back for a second: the lack of clarity (including on my part!) about what each client currently does indicates that we need to start with tests here -- this is exactly the kind of thing the conformance suite is intended for, and we should take advantage of it.
So, as an actionable proposal: we should create some test vectors ensuring that current implementations do not trust random intermediates or roots that don't also appear in the trust root, and that they can gracefully handle bundles that only contain leaves.
CC @tetsuo-cpp, since I know you were working on bundle support in the test suite.

Okay, I've been thinking about this a bit more. I think it's much clearer to view verification as a two-step process. Imagine the following structure:
- root (in TUF)
  - int1 (in TUF, pathlen=0)
    - leaf1
  - int2 (in TUF, pathlen=1)
     - int2a
        - leaf2

To verify:

Resolve roots: figure out the certificates of the Fulcio instances to trust. Fetch all the certs using TUF, then verify all the intermediates.
Sigstore verification: use only the intermediates in the certificate pool (int1, int2) as the roots for verification.

Then, the guidance for clients: clients SHOULD include the leaf up to (but not including) the Fulcio certificate in the bundle. So: leaf2->int2a, leaf1.

This has a few advantages. First, the client spec becomes much simpler: just use the provide Fulcio certs. We avoid departing from RFC 5280 by requiring any extra checks. We don't need to filter the certificates from the chain or anything (I worry a client might forget).
Second, all of the complexities about which intermediates to trust can get moved into the "policy for distributing roots." This makes sense, as that's a peculiarity of our deployment, not a fundamental part of the model. If you want to avoid surprise intermediates, you should set pathLen: 0 in the Fulcio intermediates. We should still test these complexities, but they can be in a test vector for resolving the intermediates.

Okay, I've been thinking about this a bit more. I think it's possibly clearer to view verification as a two-step process. Imagine the following structure:
...
This has a few advantages. First, the client spec becomes much simpler: just use the provide Fulcio certs. We avoid departing from RFC 5280 by requiring any extra checks. We don't need to filter the certificates from the chain or anything (I worry a client might forget).

I think this makes sense, but that would be for public good Sigstore, right? As in the BYO PKI scenario Fulcio may not even be used. Will this be clear enough in the specification?
But comparing this with the other alternative proposed in this discussion. Having a parameter (either on the CertificateAuthority or on the ArtifactVerificationOptions ) that dictates how chain building works (if we allow any intermediates to be used from the bundle or not). My understanding is that this would still be RFC 5280 style chain building, it's a question based on which material we build the chain. This is what I think you refer to here:

Second, all of the complexities about which intermediates to trust can get moved into the "policy for distributing roots."

So that I understand, your comment is mostly around how the spec clarifies what certificates goes into the bundle? For public good instance we can for sure describe what you propose, but I'm afraid that we may end up underspecified for the BYO PKI.
I'm thinking if we can describe the "default policy" for the public good instance (i.e not trust anything except the leaf cert in the bundle), and provide recommendation for a generic use case and clearly describe that each verifier MAY have their own policy on chain building/certificate verification, and those expectations should be communicated OOB before artifacts are signed. I don't think that's too much to ask that you understand how signatures are being verified before you sign them.

I wrote a bunch below but it's pretty rambly, hopefully this clears some things up but if not we can discuss.

I think this makes sense, but that would be for public good Sigstore, right? As in the BYO PKI scenario Fulcio may not even be used. Will this be clear enough in the specification?

I don't understand why you'd need the Sigstore specification for BYO PKI. It's up to you in that circumstance to figure out how to validate. If you have BYO PKI, are your certs even short-lived? We don't know that, and can't write a one-size fits all spec for you. I'm generally opposed to over-optimizing for this use case in the specifications/bundle format.

I'm thinking if we can describe the "default policy" for the public good instance (i.e not trust anything except the leaf cert in the bundle), and provide recommendation for a generic use case and clearly describe that each verifier MAY have their own policy on chain building/certificate verification, and those expectations should be communicated OOB before artifacts are signed.

I guess my point is that, for the "Sigstore Client Spec," describing it as "get some roots and some intermediates, then do some extra checks that are deployment-specific to build a path from a leaf up to the roots" is really complicated both to explain and implement. Whereas "(1) figure out which Fulcio certs you trust, and use those in the trusted certificate store; (2) you MUST validate like you'd validate any other certificate" is friendlier to openssl-style APIs (which are quite common) and moves all the complexity about figuring out what to trust over to the deployment ("Public Deployment Spec"), which is good because it's a deployment-dependent process.
Then we don't need a configuration option for how to validate, we just have to construct the trust root correctly.

So that I understand, your comment is mostly around how the spec clarifies what certificates goes into the bundle? For public good instance we can for sure describe what you propose, but I'm afraid that we may end up underspecified for the BYO PKI.

My comment is mostly about how the "Client Spec" describes the certificate validation process (section Verification -> Certificates).
The concern is that if we say "use the roots and the intermediates from the trust root, and also you can take some intermediates from the certificate chain but how you do that is totally up to you but you do have to be careful about it" is confusing (I think @woodruffw points out that this is really confusing for openssl-style verification APIs).
BUT it's pretty easy for us to say in the client spec "just validate like normal, chaining up to the Fulcio cert that you got securely out of band."
Then, if we want to enforce the rule "only use intermediates that come from TUF," we do that when describing certificate distribution (which is a property of the Public Good Deployment):

Use TUF to get all the certs
Then validate the Fulcio intermediates up to the Fulcio roots.
Then just use the Fulcio intermediates as the Fulcio certificates as in the client spec.

I don't think that's too much to ask that you understand how signatures are being verified before you sign them.

That's why a simple rule like "always chain up to the Fulcio cert, ignore everything above that" is nice. That should always be true. Then "how signatures are being verified" is part of "get the keys securely, how you do that is out of scope."
I don't know if we're disagreeing really, I just really want the "Client Spec" to be simple and prescriptive about this question and the quirks of how this actually works in Sigstore can go in the "Public Deployment Spec."

I don't know if we're disagreeing really, I just really want the "Client Spec" to be simple and prescriptive about this question and the quirks of how this actually works in Sigstore can go in the "Public Deployment Spec."

No, I believe we are trying to achieve the same thing :)
It's the BYO PKI adds confusion IMHO. And I think that can be left outside of the "Client Spec", i.e if you have BYO PKI you are on your own.
The question as I see it is if we have a language in the comments in this repo that makes BYO PKI conflicting/confusing, would we update that, or keep the language in this repo compliant with the "Client Spec"?
If the language is more general in this repo we can "tighten/clarify" in the "Client spec".

It's the BYO PKI adds confusion IMHO. And I think that can be left outside of the "Client Spec", i.e if you have BYO PKI you are on your own.

👍

The question as I see it is if we have a language in the comments in this repo that makes BYO PKI conflicting/confusing, would we update that, or keep the language in this repo compliant with the "Client Spec"?
If the language is more general in this repo we can "tighten/clarify" in the "Client spec".

IMO, we should prioritize the full Sigstore flow in this repo.
Then, we can have guidance about BYO PKI which basically says "hey, use this Client Spec flow with modifications X and Y and you might want to use the protobuf-specs bundle, disregarding Z." We don't want to leave BYO PKI folks in the dark, but at the same time I don't think we can anticipate every need they might have.

IMO, we should prioritize the full Sigstore flow in this repo.

Then, we can have guidance about BYO PKI which basically says "hey, use this Client Spec flow with modifications X and Y and you might want to use the protobuf-specs bundle, disregarding Z." We don't want to leave BYO PKI folks in the dark, but at the same time I don't think we can anticipate every need they might have.

💯

LGTM on that, let’s make that explicit in the documentation.

Okay, so to summarize the expected doc changes here:

Add a MUST NOT indicating that the cert chain must not contain a root CA
Add a SHOULD NOT indicating that the cert chain should not contain intermediate CAs that should appear in an independent root of trust
Add a MUST indicating that verifying clients must validate the chain carefully to ensure that it upholds the invariants above (maybe this belongs somewhere else?)

If that sounds good, then I can create a PR and close this out!

LGTM, (3) seems necessary to avoid breaking backwards compatibility. Should it be MUST be prepared to handle old bundles? I think our intention is "new clients SHOULD, existing clients with significant usage MUST"?

Yes, good point!


Cmake Issue Array Fire

Ubuntu 20.04
Cmake 3.16.3
GCC 9.4.0
Fresh system install, only trying to build Flashlight on it currently.
KENLM and MKL paths set.
Python binding install went well.
Using command
cmake .. -DFL_BACKEND=CPU -DCMAKE_BUILD_TYPE=Release -DFL_BUILD_TESTS=OFF -DFL_BUILD_EXAMPLES=OFF -DFL_BUILD_APP_ASR=ON
Gets me to
-- ArrayFire found (include: /usr/include, library: )
CMake Error at flashlight/fl/tensor/backend/af/CMakeLists.txt:19 (message):
ArrayFire CPU not found: cannot build CPU backend
Call Stack (most recent call first):
flashlight/fl/tensor/CMakeLists.txt:6 (include)
flashlight/fl/CMakeLists.txt:73 (include)
CMakeLists.txt:217 (include)
Not sure where to go from here.
Arrayfire was installed via...
sudo apt-get update
sudo apt-get install libarrayfire-dev
EDIT :
Since then I have also tried downloading the file from ArrayFire directly, running their .sh setup script
ArrayFireSetup.sh --prefix=/opt
echo /opt/arrayfire/lib64 > /etc/ld.so.conf.d/arrayfire.conf
ldconfig
Reran the setup, same error.
And I still get the same error.

If you can confirm that libafcpu is present on your system post-install? If it is, passing the ArrayFire_DIR option to CMake is required. I assume you've solved this based on your follow-up issues, but feel free to re-open if not.


Add FocusTrap to Dropdown

@cehsu gave me the accessibility bug. This adds a focus trap to Dropdowns which forces tabbing to stay trapped within the dropdown menu when open.

Codecov Report

Merging #27 into fix/dropdown-blur will increase coverage by 4.3%.
The diff coverage is 61.11%.

@@                 Coverage Diff                  @@
##           fix/dropdown-blur      #27     +/-   ##
====================================================
+ Coverage              70.54%   74.85%   +4.3%
====================================================
  Files                     12       13      +1
  Lines                    146      167     +21
  Branches                  21       22      +1
====================================================
+ Hits                     103      125     +22
- Misses                    31       33      +2
+ Partials                  12        9      -3

Impacted Files
Coverage Δ

src/Card/Card.js
100% <ø> (ø)
:arrow_up:

src/utils.js
100% <100%> (ø)
:arrow_up:

src/Button/Button.js
66.66% <100%> (ø)
:arrow_up:

src/Dropdown/Dropdown.js
54.54% <53.65%> (+18.18%)
:arrow_up:

src/hooks/useKeyPress.js
80% <80%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ca64b7...ef8d0b7. Read the comment docs.


separate into demo and cli scripts

Fixes #177.
Need to make separate scripts for a demo and CLI.
Review Checklist for Source Code Changes

[ ] Does pip install still work?
[ ] Have you written a unit test for any new functions?
[ ] Do all the units tests run successfully?
[ ] Does adler run successfully on a test set of input files/databases?
[ ] Have you used black on the files you have updated to confirm python programming style guide enforcement?

Okay so I think this is ready to go. I have now got adler_run.py into a usable state and I have added some CLI args to add functionality namely --plot_show which allows interactive display of plots and --phase_model which allows yout to select which sbpy phase model to use on the data.

As I had added CLI args and also new AdlerData columns (modelFitMjd) I also had to update the unit tests. As I was updating the test data anyway I decided to script this so that it's easier when we add more columns to AdlerData, just run tests/data/adler_run_test_data.py. Getting unit tests working took a long time, first because it is difficult to assert compare dicts that are almost equal (small changes are expected due to reading/writing test data to file and display precision). I tried pytest.approx but this cannot handle comparison of nans. The solution so far has been to compare key values directly #181

Second the current behaviour of AdlerData.write_row_to_database is now a problem: when a new model2 is calculated it is added as a new timestamped row with nan entries for the previously calculated model1 values. Therefore retrieving the most recent row will not have the most recent values for both model1 and model2. We will need to change this behaviour such that an object generally has a single row and the relevant model value columns are updated when writing.


Revert "fix: Revert AVD-AWS-0342 policy (#1309)"

This reverts commit b5376b04a53b335467d26ab787fec669d39b0fc9.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.Phaedra Solutions seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.You have signed the CLA already but the status is still pending? Let us recheck it.

@r-khurram  can you sign the CLA?


Allow nested locale config files

When working with very large websites, having all the i18n config in one file is overwhelming. From a translation perspective, it's easier to work with individual files for each view.
I've added a feature to support this:
https://github.com/shaiguitar/haml-i18n-extractor/pull/23

Hey @jeznag - thanks! I haven't used this gem in a while so you probably have the better perspective on usability than me at this point.


ServerContext: 1 leaked_rewrite_drivers on destruction +mod_pagespeed

Hi,

  I am getting below error for mod_pagespeed in the apache logs, Can any one hlep me in getting this issue resolved?

[Fri Jan 02 05:24:33 2015] [error] [mod_pagespeed <IP_ADDRESS>-4056 @3656]
ServerContext: 1 leaked_rewrite_drivers on destruction
[Fri Jan 02 05:24:33 2015] [error] [mod_pagespeed <IP_ADDRESS>-4056 @3639]
ServerContext: 1 leaked_rewrite_drivers on destruction
[Fri Jan 02 05:58:29 2015] [error] [client <IP_ADDRESS>] unable to include
"/content/smu/_jcr_content/headeriparsys/globalnav/socialconnect2.nocache.html/m
anipal/components/socialconnect" in parsed file
/var/www/html/content/smu/error/filenotfound.html, referer:
http://smu.edu.in/smims.html/notice_view.php?t=p&news=324
[Fri Jan 02 05:58:34 2015] [error] [client <IP_ADDRESS>] unable to include
"/content/smu/_jcr_content/headeriparsys/globalnav/socialconnect2.nocache.html/m
anipal/components/socialconnect" in parsed file
/var/www/html/content/smu/error/filenotfound.html, referer:
http://smu.edu.in/smims.html/notice_view.php?t=p&news=324
[Fri Jan 02 05:58:37 2015] [error] [client <IP_ADDRESS>] unable to include
"/content/smu/_jcr_content/headeriparsys/globalnav/socialconnect2.nocache.html/m
anip

Original issue reported on code.google.com by<EMAIL_ADDRESS>on 2 Jan 2015 at 11:22

Discussed in https://groups.google.com/forum/#!searchin/mod-pagespeed-discuss/ServerContext$3A$201$20leaked_rewrite_drivers$20on$20destruction/mod-pagespeed-discuss/EdJ-Zq9Kl50/N9UyWyseeOYJ
Should we downgrade this to info?


dar: 2.7.10 -> 2.7.13

Automatic update generated by nixpkgs-update tools. This update was made based on information from https://repology.org/project/dar/versions.
meta.description for dar is: Disk ARchiver, allows backing up files into indexed archives
meta.homepage for dar is: http://dar.linux.free.fr
Updates performed

Version update

To inspect upstream changes
Impact

Checks done (click to expand)

built on NixOS
The tests defined in passthru.tests, if any, passed
found 2.7.13 with grep in /nix/store/063y77vlpwiczabwjh6qhcy9wrz79j07-dar-2.7.13
found 2.7.13 in filename of file in /nix/store/063y77vlpwiczabwjh6qhcy9wrz79j07-dar-2.7.13

Rebuild report (if merged into master) (click to expand)

1 total rebuild path(s)

1 package rebuild(s)

First fifty rebuilds by attrpath
dar

Instructions to test this update (click to expand)

Either download from Cachix:
nix-store -r /nix/store/063y77vlpwiczabwjh6qhcy9wrz79j07-dar-2.7.13 \
  --option binary-caches 'https://cache.nixos.org/ https://nix-community.cachix.org/' \
  --option trusted-public-keys '
  nix-community.cachix.org-1:mB9FSh9qf2dCimDSUo8Zy7bkq5CX+/rkCWyvRCYg3Fs=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The Cachix cache is only trusted for this store-path realization.)
For the Cachix download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.
Or, build yourself:
nix-build -A dar https://github.com/r-ryantm/nixpkgs/archive/96ac708b24922904571962b4113e4040a68d496b.tar.gz

Or:
nix build github:r-ryantm/nixpkgs/96ac708b24922904571962b4113e4040a68d496b#dar

After you've downloaded or built it, look at the files and if there are any, run the binaries:
ls -la /nix/store/063y77vlpwiczabwjh6qhcy9wrz79j07-dar-2.7.13
ls -la /nix/store/063y77vlpwiczabwjh6qhcy9wrz79j07-dar-2.7.13/bin

Pre-merge build results
We have automatically built all packages that will get rebuilt due to
this change.
This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.
Result of nixpkgs-review run on x86_64-linux 1

1 package built:

dar

Maintainer pings
cc @Izorkin for testing.

@ofborg eval

Result of nixpkgs-review pr 264271 run on aarch64-darwin 1

2 packages built:

dar
dar.dev

I'll try to look at the failure on staging-next, probably it suffices to drop llvmPackages_12 there since LLVM 16 is now the default.


fix: Disabling dev tool make the project crash

🐛 The bug
When you create a new project with :
npx nuxi@latest init test

then run :
npx nuxi devtools disable

the run the project with : npm run dev
you get this error :
 ERROR  Internal server error: Failed to resolve import "virtual:vue-inspector-path:Overlay.vue" from "node_modules/vite-plugin-vue-inspector/src/load.js?v=51e81404". Does the file exist?       15:45:51
  Plugin: vite:import-analysis
  File: /Users/landry/Desktop/test/node_modules/vite-plugin-vue-inspector/src/load.js?v=51e81404:4:18
  2  |
  3  |  import * as Vue from 'vue'
  4  |  import App from 'virtual:vue-inspector-path:Overlay.vue'
     |                   ^
  5  |  import inspectorOptions from 'virtual:vue-inspector-options'
  6  |  const CONTAINER_ID = 'vue-inspector-container'
      at formatError (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:50830:46)
      at TransformContext.error (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:50824:19)
      at normalizeUrl (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:66128:33)
      at async file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:66283:47
      at async Promise.all (index 1)
      at async TransformContext.transform (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:66204:13)
      at async Object.transform (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:51139:30)
      at async loadAndTransform (file:///Users/landry/Desktop/test/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:53894:29)

🛠 To reproduce
https://github.com/Arcelone/devtoolsbug
🌈 Expected behavior
That the project works properly without the devtools.
ℹ Additional context
.nuxtrc contain just this telemetry.enabled=false
version :

nvm : 0.39.7 (installed with curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.7/install.sh | bash)
node : v20.13.1
npm : 10.8.0
Nuxt : 3.11.2
Nitro 2.9.6

No "system" version of node installed. I have tried with but doesn't solve anything.
If you create the project with :
npx nuxi@latest init content-app -t content

and the disable with :
npx nuxi devtools disable

everything is working well.

nuxt.config.ts
export default defineNuxtConfig({
  vite: {
    server: {
      hmr: {
        overlay: false,
      },
    },
  },
})

@Arcelone I just did your reproduction steps and I got no error, worked as expected

This behavior is fix withe 3.12 version release. 👍🏻

I'm getting this as well, 3.12.4

Can confirm, I'm getting the same error in 3.12.4.

Can confirm, I'm getting the same error in 3.13.0 also.
 ERROR  Internal server error: Failed to resolve import "virtual:vue-inspector-path:Overlay.vue" from "../../node_modules/vite-plugin-vue-inspector/src/load.js?v=dc493e27". Does the file exist?                                                                                3:58:51 PM
   Plugin: vite:import-analysis
   File: ./node_modules/vite-plugin-vue-inspector/src/load.js?v=dc493e27:4:18
   2  |
   3  |  import * as Vue from 'vue'
   4  |  import App from 'virtual:vue-inspector-path:Overlay.vue'
       |                   ^
   5  |  import inspectorOptions from 'virtual:vue-inspector-options'
   6  |  const CONTAINER_ID = 'vue-inspector-container'

Get this in latest Chrome but not in Firefox on a mac.

Still getting this issue with 3.13.2

Same. 3.13.2

Fix with this version :

Nuxt Version:     3.13.2
CLI Version:      3.14.0
Nitro Version:    2.9.7

If any one can confirme ? That I can close the issue.

same! 3.13.2

Same problem

So, on 3.13.2, i managed to get it going again by adding this explicitly on nuxt.config.ts:
devtools: {
enabled: false,
}
also, i read somewhere that nuxt-security might be causing the issue, but I can't confirm. I've got nuxt-security set up, with the headers settings to make devtools work, but if i explicitly disable them with the code above, it doesn't crash.

Problem went away on its own.. not really sure what happened.


Added West Europe URL and updated ms.date

Hi @tedway, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

It looks like you're working at Microsoft (tedway). If you're full-time, we DON'T require a contribution license agreement.

If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

@tedway - Please resubmit this update through the private repo. You can access the private version of the article by adding -pr to the repo name in the URL. Your article needs to run through the current private repo validation.


MatTextField Outlined Label not visible when value set while component hidden

Hi,
MatTextField label is invisible after opening expansion panel.

Please check this fiddle.
https://blazorfiddle.com/s/lehxzahb

closed as duplicate https://github.com/SamProf/MatBlazor/issues/255


[Bug] Not supported JVM GC  Metrics in ZGC?

Search before asking

[X] I had searched in the issues and found no similar issues.

Apache SkyWalking Component
UI (apache/skywalking-booster-ui)
What happened
UI can not display JVM GC Time and Count metrics when server use zgc.
Look the picture.

What you expected to happen
UI can  display JVM GC  metrics when server use zgc.
How to reproduce

JDK 17 and JVM params:  -XX:+UseZGC
Skywalking Version: 9.5.0(Agent,Server,UI are all 9.5.0)

Anything else
No response
Are you willing to submit a pull request to fix on your own?

[ ] Yes I am willing to submit a pull request on my own!

Code of Conduct

[ ] I agree to follow this project's Code of Conduct

Please don't report a bug for a thing never declared support. This is a misinformation.
AFAIK, ZGC in JDK17 is not supported in Java agent 9.0, which is still on release voting. https://github.com/apache/skywalking-java/blob/main/changes/changes-9.0.0.md

Please don't report a bug for a thing never declared support. This is a misinformation. AFAIK, ZGC in JDK17 is not supported in Java agent 9.0, which is still on release voting<EMAIL_ADDRESS>
I tagged the issue bug because I found that the pr had been merged.

https://github.com/apache/skywalking/issues/8411
https://github.com/apache/skywalking/pull/8425
I found relevant discussion in 2022, so I subconsciously thought this feature was supported.

https://github.com/apache/skywalking/pull/8425 is indicated its milestone, aka, release version.

Merging doesn't mean you could use it unless you compiled from the source codes by yourself.


Allow using earlier recipe parameters in later parameters without having them in outer scope

For some reason this works:
a := ''
target a *b=a:
  echo {{ b }}

But this doesn't:
a := ''
target a *b=a:
  echo {{ b }}

It should be possible to use the value of the a parameter without having a in the outer scope.

Fixed in #2382, this was so close to working and so simple to fix, it even had a test which checked that it was an error. I hope past me didn't have a really good reason for not allowing this.


Alternate direction are not showing?

Alternate direction are showing with same color as main route color I have set in map, how to show them with different color

Follow this,
https://github.com/akexorcist/Android-GoogleDirectionLibrary/blob/master/app/src/main/java/com/akexorcist/googledirection/sample/AlternativeDirectionActivity.java


Cannot import exported resource template

When I export any resource template from dev in order to import locally, I get:
The profile you provided was not in an accepable format: TypeError: content.Profile is undefined
Should importing be possible?

Non-issue


added info and texinfo targets to Makefile

Sphinx now supports texinfo builds (makes for happy emacs users).

Thanks!


Issue #375: Validate Enum literals

Also case sensitivity is an interesting feature, see chapter 5.21 of the IDL 3.5 specification:
IDL identifiers are case insensitive; that is, two identifiers that differ only in the case of their characters are considered
redefinitions of one another. However, all references to a definition must use the same case as the defining occurrence.
This allows natural mappings to case sensitive languages.

Thank you for the comments. We can support these cases when NGC wants them to be implemented.


Reland "redux of a change to use new engine APIs for Flutter.Frame events"

Reverts flutter/flutter#34514 - the bots weren't failing due to this.
Will land on red to rekick bots that should now be in order.

Just seeing this now -thanks for re-landing.


Find and replace Dataspace with "Data Space" in all documentation

Which part of the repository is your change request related to?
In all parts of the IDS Github, to find and replace "Dataspace" with "Data Space".

Done


Install cmake config files

Note: this does still not enable me to find Vc for some reason, so this
patch is no good and just a basis for discussion.
I'm a bit unsure how the file structure should be to work.
find_package(Vc 1.2.0)
gives me:
-- Using CMake version: 3.6.2
CMake Error at /opt/Vc/cmake/VcConfig.cmake:17 (include):
include could not find load file:
Vc_CMAKE_MODULES_DIR-NOTFOUND/VcMacros.cmake

Call Stack (most recent call first):
/home/frederik/dev/kde/krita/cmake/modules/FindVc.cmake:29 (find_package)
CMakeLists.txt:15 (find_package)
CMake Error at /opt/Vc/cmake/VcConfig.cmake:22 (vc_set_preferred_compiler_flags):
Unknown CMake command "vc_set_preferred_compiler_flags".
Call Stack (most recent call first):
/home/frederik/dev/kde/krita/cmake/modules/FindVc.cmake:29 (find_package)
CMakeLists.txt:15 (find_package)
-- Configuring incomplete, errors occurred!

Thanks for the report. This was broken by c4d6b410ab533a8db672b24cd7dbf2cc07882cce. There's an else branch that I accidentally removed.

Thanks! I was wondering how that worked before ;) Should I abandon this? I might find time to look into the change you pointed out, but I'm not sure when.

I'll fix it. Thanks


Vina-GPU runtime error: "./OpenCL/src/kernels/code_head.cpp Error: failed to open file"

I can successfully build Vina-GPU on Ubuntu 20.04. However when I run Vina-GPU the error happened.
I re-compiled with -DOPENCL_3_0/-DOPENCL_2_0/-DOPENCL_1_2 in MakeFile, all can be built successfully but got the same runtime error like below. Anyone can help on this?
Using single ligand docking mode
Output will be ./039/cmpd-39_out.pdbqt
Reading input ... done.
Setting up the scoring function ... done.
Using heuristic search_depth
Analyzing the binding site ... done.
GPU Platform: NVIDIA CUDA
GPU Device: Tesla V100-PCIE-16GB
Using random seed: 970523110
Build kernel 1 from sourceError: failed to open file
: ./OpenCL/src/kernels/code_head.cppError: failed to open file
Segmentation fault (core dumped)
Thanks,
Regards,
Raymond,

I had a similar problem.
WARNING: The search space volume > 27000 Angstrom^3 (See FAQ)
Using single ligand docking mode
Output will be lig_out.pdbqt
Reading input ... done.
Setting up the scoring function ... done.
Search_depth is fixed to 200
Analyzing the binding site ... done.
GPU Platform: NVIDIA CUDA
GPU Device: NVIDIA GeForce RTX 4090
Using random seed: -564014416
Build kernel 1 from sourceError: failed to open file
: ./OpenCL/src/kernels/code_head.clError: failed to open file
: ./OpenCL/src/kernels/kernel1.cl
An error occurred: basic_string::_M_construct null not valid.

Hi, please put the Vina-GPU executable file in the root dir of Vina-GPU, so that Vina-GPU could find the file path (which is ./OpenCL/src/***)

I have it installed as a part of sbgrid. Still the same issue persists!


CZML can't reference position with intervals

If one entity has a composite position (a position using time intervals), and another entity attempts to reference that position, the render loop throws.  Here's some CZML:
[
  {
    "id":"document",
    "version":"1.0",
    "clock":{
      "interval":"2017-01-05T11:00:00Z/2017-01-05T11:59:59Z",
      "currentTime":"2017-01-05T11:29:59.5Z",
      "multiplier":60,
      "range":"LOOP_STOP"
    }
  },
  {
    "id":"Test Reference Object",
    "availability":"2017-01-05T11:00:00Z/2017-01-05T11:59:59Z",
    "name":"Test Reference Object",
    "path":{
      "show":true,
      "width":2,
      "resolution":300,
      "leadTime":0,
      "trailTime":172800,
      "material":{
        "solidColor":{
          "color":{
            "rgba":[
              255,255,0,255
            ]
          }
        }
      }
    },
    "position":{
      "reference":"Test Main Object#position"
    }
  },
  {
    "id":"Test Main Object",
    "availability":"2017-01-05T11:00:00Z/2017-01-05T11:59:59Z",
    "name":"Test Main Object",
    "label":{
      "text":"Test Main Object",
      "font":"bold 32px \"Droid Sans\",sans-serif",
      "style":"FILL_AND_OUTLINE",
      "scale":0.5,
      "pixelOffset":{
        "cartesian2":[
          5,0
        ]
      },
      "horizontalOrigin":"LEFT",
      "verticalOrigin":"CENTER",
      "fillColor":{
        "rgba":[
          255,255,0,255
        ]
      },
      "outlineColor":{
        "rgba":[
          0,0,0,255
        ]
      },
      "outlineWidth":2.4
    },
    "point":{
      "pixelSize":7,
      "color":{
        "rgba":[
          255,255,0,255
        ]
      }
    },
    "position":[
      {
        "interval":"2017-01-05T11:00:00Z/2017-01-05T11:59:59Z",
        "interpolationAlgorithm":"LAGRANGE",
        "interpolationDegree":5,
        "referenceFrame":"INERTIAL",
        "epoch":"2017-01-05T11:00:00Z",
        "cartesian":[
          0,10775966.0,-40747218.0,-86707.0,
          3599,20984737.0,-36550703.0,-135028.0
        ]
      }
    ]
  }
]

The error thrown is:
TypeError: Cannot read property 'length' of undefined
    at subSampleCompositeProperty (http://c1200610-00.stk.com/Cesium/Source/DataSources/PathVisualizer.js:208:38)
    at reallySubSample (http://c1200610-00.stk.com/Cesium/Source/DataSources/PathVisualizer.js:246:21)
    at subSample (http://c1200610-00.stk.com/Cesium/Source/DataSources/PathVisualizer.js:263:22)
    at PolylineUpdater.updateObject (http://c1200610-00.stk.com/Cesium/Source/DataSources/PathVisualizer.js:369:30)
    at PathVisualizer.update (http://c1200610-00.stk.com/Cesium/Source/DataSources/PathVisualizer.js:472:32)
    at DataSourceDisplay.update (http://c1200610-00.stk.com/Cesium/Source/DataSources/DataSourceDisplay.js:260:41)
    at Viewer._onTick (http://c1200610-00.stk.com/Cesium/Source/Widgets/Viewer/Viewer.js:1483:49)
    at Event.raiseEvent (http://c1200610-00.stk.com/Cesium/Source/Core/Event.js:147:30)
    at Clock.tick (http://c1200610-00.stk.com/Cesium/Source/Core/Clock.js:309:21)
    at CesiumWidget.render (http://c1200610-00.stk.com/Cesium/Source/Widgets/CesiumWidget/CesiumWidget.js:681:43)

To work around this, copy the position block from the main entity above, and replace the position reference in the other entity with a copy-and-paste of the main position block.  Then the CZML will work fine.  But, there's no easy way to tell the STK Components CZML writer to use this workaround.

From my point of view at least, this is a priority :smile:

The fix for this should be trivial, the problem is in reallySubSample in PathVisualizer.js, it unwraps the inner property but then tries to sample the ReferneceProperty instead of the unwrapped property. However, there are dragons here and I found at least 2 other path bugs (one is kind of tricky) while looking at this.  I'll see what I can do.


Creates files with trailing underscore.

Moved from https://github.com/metanorma/relaton-iso/issues/58
In geolexica.org , both of these files are created in the cache:

iso_12620_1999.xml
iso_12620_1999_.xml

@andrew2net it seems that the problem is Relaton thinks a cached entry doesn't exist but it already does. Does this fix that?

@ronaldtse this fix only trailing underscore in filenames.

Got it, so the remaining task is #53.


Someting wrong with the Summary

I saw the code "var SummaryLength = 70" in the hugo/helpers/content.go, but in my case, the summary contains all the text in the markdown file except tags(I didn't use the SummaryDivider).

The auto summary has some word logic in it that may fail for languages where wordsplit by space doesn't make sense.
What language are you writing in?
/cc @RickCogley

thanks for the heads-up @bep

I use Chinese, Thanks.

The summary i 70, but it tries to finish the sentence. What is the end-of-sentence marker in Chinese?

。, it equals "." in English.

It is the same with Japanese, U+3002 Ideographic Full Stop is used as end-of-sentence. U+3001 ideographic comma is used as comma in English.
These are most common characters in Chinese and Japanese but there are some more variation, e.g. full width and half width etc.


Prerequisites for 0.7 release

Updating Travis to build properly (including using standardized Uber OSS template)
Migrating duplicate Gradle files into shared space
Updating dependencies for Java SDKs
Moving to Github first development model

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


TODO: Add some post-install checks

Partial transfer of https://github.com/ocaml/opam-repository/issues/17816
On top of this, we could add other checks. I don't have any other ideas on top of my head right now but something interesting to do might pop-up at some point.
cc @bobot

@kit-ty-kate mam i want to work on this issue? Can you please guide me little for the same.

Hey @ps-19, welcome to the community sorry that we've all missed this! Are you still interested in working on this issue? Have you already installed OCaml and compiled this project?


Xu_hooks_api' not found

im problem
Fatal error: Class 'Xu_hooks_api' not found in /home/xxxxxx/public_html/system/application/libraries/xu_api.php on line 74
please fix this my problem

this project is long since dead.


Null metrics in sweep table results, but not in charts[App]

Describe the bug
In my sweeps, most of the metrics in the sweep table are shown as '-' but the metric data in the charts is complete and correct.
Expected behavior
Barring an error, I expect the sweeps table to show the metrics for easy comparison
Screenshots
Some runs in the sweep tables are missing data:

Charts still contain all the data. Please note all runs ran the full # of epochs, but they are using differing degrees of batch_accumulation which is why they completed a different number of steps.

Operating System

Ubuntu 20.04.3 LTS
Browser Chrome 92.0.4515.159
WandB 0.12.1

Additional context
Add any other context about the problem here.

@rbracco can you share a link to the sweep where this is happening for you?

https://wandb.ai/madeupmasters/thunder_train-train_scripts/sweeps/vc3m8mrn but it's set to private. Is there a way I can either share logs or grant you access without making it fully public?


RuntimeError: merge_sort: failed to synchronize: cudaErrorIllegalAddress: an illegal memory access was encountered

Hi!
Thanks for your excellent work! I followed the README to train MaskPLS. However, I encountered the following error during the first iteration.
I have checked outputs, loss_dict, total_loss, and they all seem to be fine.
My environment is torch==1.10.1+cu111, and the rest are the same as in the requirement.txt file.
Could you please help me understand how this problem arose and how to solve it? Thanks in advance!
Traceback (most recent call last):
  File "scripts/train_model.py", line 81, in <module>
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "scripts/train_model.py", line 40, in main
    "experiments/" + cfg.EXPERIMENT.ID, default_hp_metric=False
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 737, in fit
    self._call_and_handle_interrupt(
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 682, in _call_and_handle_interrupt
    return trainer_fn(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 772, in _fit_impl
    self._run(model, ckpt_path=ckpt_path)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 1194, in _run
    self._dispatch()
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 1274, in _dispatch
    self.training_type_plugin.start_training(self)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/plugins/training_type/training_type_plugin.py", line 202, in start_training
    self._results = trainer.run_stage()
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 1284, in run_stage
    return self._run_train()
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/trainer/trainer.py", line 1314, in _run_train
    self.fit_loop.run()
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/base.py", line 145, in run
    self.advance(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/fit_loop.py", line 234, in advance
    self.epoch_loop.run(data_fetcher)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/base.py", line 145, in run
    self.advance(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/epoch/training_epoch_loop.py", line 193, in advance
    batch_output = self.batch_loop.run(batch, batch_idx)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/base.py", line 145, in run
    self.advance(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/batch/training_batch_loop.py", line 88, in advance
    outputs = self.optimizer_loop.run(split_batch, optimizers, batch_idx)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/base.py", line 145, in run
    self.advance(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/optimization/optimizer_loop.py", line 215, in advance
    result = self._run_optimization(
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/optimization/optimizer_loop.py", line 259, in _run_optimization
    closure()
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/optimization/optimizer_loop.py", line 160, in __call__
    self._result = self.closure(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/optimization/optimizer_loop.py", line 155, in closure
    self._backward_fn(step_output.closure_loss)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/loops/optimization/optimizer_loop.py", line 327, in backward_fn
    self.trainer.accelerator.backward(loss, optimizer, opt_idx)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/accelerators/accelerator.py", line 311, in backward
    self.precision_plugin.backward(self.lightning_module, closure_loss, *args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/plugins/precision/precision_plugin.py", line 91, in backward
    model.backward(closure_loss, optimizer, *args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/pytorch_lightning/core/lightning.py", line 1434, in backward
    loss.backward(*args, **kwargs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/torch/_tensor.py", line 307, in backward
    torch.autograd.backward(self, gradient, retain_graph, create_graph, inputs=inputs)
  File "/home/yangyu/envs/PS/lib/python3.8/site-packages/torch/autograd/__init__.py", line 154, in backward
    Variable._execution_engine.run_backward(
RuntimeError: merge_sort: failed to synchronize: cudaErrorIllegalAddress: an illegal memory access was encountered

I changed the torch version to torch==1.10.1+cu113 and the problem is fixed. Thanks!


potential changes to BW config in docs

The config in the 1.0 docs says
config = Config(
    executors=[
        HighThroughputExecutor(
            label="bw_htex",
            cores_per_worker=1,
            worker_debug=False,
            address=address_by_hostname(),
            provider=TorqueProvider(
                queue='normal',
                launcher=AprunLauncher(overrides="-b -- bwpy-environ --"),
                scheduler_options='',  # string to prepend to #SBATCH blocks in the submit script to the scheduler
                worker_init='',  # command to run before starting a worker, such as 'source activate env'
                init_blocks=1,
                max_blocks=1,
                min_blocks=1,
                nodes_per_block=2,
                walltime='00:10:00'
            ),
        )

    ],

)

Two possible additions are:
1 - explain how to use the XK nodes, with an option like
                scheduler_options='#PBS -l nodes=1:ppn=16:xk
which would also require explaining what options here overwrite other parts of the config
2 - explain that virtualenv is the recommended environment for Python on BW, which leads to
                worker_init='module load bwpy; source $VIRTUALENVPATH/bin/activate',
or something similar

@ZhuozhaoLi, I hope that perhaps you can help put this in as a PR - I'm not exactly sure how best to put this idea of options and potential text into a config object in the docs...

@danielskatz – can we close this issue since BW is no longer in use?


Add new API functions

These were extracted from #2819 for easier merging.

Thanks @borisyankov ! These generally look good. A few small points:

jsdoc should go on the functions (not the types), so hover works; and with no blank line in between, so it doesn't end up getting separated later.
Let's use exact object types; and spread rather than intersection-type syntax, because the latter seems to have puzzling semantics in Flow (even in the non-exact case, where as far as I can see they ought to be equivalent.)
Let's put some quick jsdoc on the UserProfile type; even just a link to the get-profile API doc would be helpful.
The doc for create-user says the user requesting it has to be an organization administrator -- it's not for anonymously creating an account from scratch. So (a) we should pass a real auth after all, and (b) as that's (empirically :slightly_smiling_face: ) a surprising requirement, let's mention it in the jsdoc for the function.

Feedback implemented. Ready for a final review.

Thanks @borisyankov -- merged!
I made just two changes:

The commit message on createUser hadn't been updated to reflect our learning that it's for org admins' use :stuck_out_tongue: ; I updated it.
I added a commit on top adjusting getMessageHistory's type, and adding a bit of jsdoc, based on my reading of its API doc page.


new invokeCallback method

Includes new invokeCallback method and associated test.
This pull request is motivated by my personal difficulty in using highland with mongoose, the mongoDB odm.  Common mongoose async operations like Model.findOne({ _id: 'abc123' }, cb) are context dependent, and don't work with _.wrapCallback.
I've been extending highland with this method in my personal projects, but it's possible that I'm missing a simpler solution using highland's existing library.

I actually only encountered this problem with the Amazon SQS SDK yesterday, I just ended up wrapping it in Bluebird's promisifyAll but I guess it would be nice to be able to do this natively.

I've always just used .bind for this, like in some of the examples here: https://github.com/caolan/highland/issues/153
People don't like bind because it can be slow, but I've never had a situation where it actually had any perf impact.

Given the existence of Function.prototype.bind, I don't think this needs to be in highland. bind also lets you bind arbitrary numbers of leading arguments, while this only lets you bind the context.
I've only ever seen one instance where bind caused perf issues, and that's when it's used in the consume code path in Highland.

@LewisJEllis yes _.wrapCallback(context.method.bind(context)) works, but it is very verbose.  I don't think its great to force users to write e.g. _.wrapCallback(Product.FindOne.bind(Product)) (I ran into this problem while using mongoose).  I can add an error test case.

@maxgurewitz My concern is that I don't think it's something that belongs in Highland. wrapCallback provides the interop with node-style callbacks, and your invokeCallback just wraps Function.prototype.bind (in functionally). I don't think it's worth it to add another function just for that.
That said, I think I would be OK with changing wrapCallback to take an optional context object like bluebird's promisify, if you'd like to make another PR.
An equivalent to promisifyAll (streamifyAll?) would be cool too...and significant enough to be worth adding.

I'm quite familiar with the mongoose use case. _.invokeCallback is limited to a single use case, where we just want to bind the context. Using Function.prototype.bind also allows us bind leading arguments, as @vqvu mentioned, and works with _.nfcall. It's also a native JS idiom; there's no need to reinvent the wheel in highland terms when Javascript has a mechanism for it.

The promisifyAll code in Bluebird is absolutely insane, it would be a pretty big undertaking to do something like streamifyAll.


Improve exception handling

Two commits, in case you wish to cherry-pick:

5a07c85 simply "hides" the PHP Warning on a 404 if an app is not found, and directly returns an empty array
df9bb95 introduces two new array keys and applies them where appropriate:

success: set to 1 if data was successfully retrieved, including all other keys as before
message: only added if success==0, giving details on what failed

I was tempted to add a third commit with some cleanup – but knowing my own code I thought you might wish to keep the "notes" :wink:
PS: Would love if summary and featureGraphic could be extracted as well, but it seems those are not contained in the scraped page. But if you wish I can check my old scraper for additional details like price, votes etc – just let me know.

Good idea.
Merged, Thank you. :+1:


task: 1664 - enable save, if when non-terminal

Reworked button state logic, and guarded edge case where empty training dialogs

:tada: This PR is included in version 0.324.28 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Automated CI

There is currently no automated testing and deployment of the codebase. For this to be considered, a CI/CD setup (e.g., GitHub Actions) should be created that tests PRs, creates releases, and deploys to PyPI
cc https://github.com/openjournals/joss-reviews/issues/2533

I have completed writing tests using standard testing frameworks based on your suggestions. I should be able to complete the CI/ CD setup within this week.

The repository now supports automated testing and deployment. I have created all the CI/CD workflows as per your suggestion. Kindly let me know if further modifications are required or it is fine to close the issue tracker.

Why is there both .github/workflows/buildPython.yml and .github/workflows/testsPython.yml? They're almost duplicates

Also, .github/workflows/testsCpp.yml should have a separate step for building and testing the C++. It's currently lumped into - name: Install dependencies which makes it impossible to read: https://github.com/kaustubh-sadekar/OmniCV-Lib/runs/1242220788?check_suite_focus=true


fix: allow let:index on Template

This could also be released before 7.0

I added the required changes to the template engine to expose index and merged it on #232


Proposal: Use Descriptive Variable Names

Hello,
While looking into some other potential improvements to the project, I noticed a simple opportunity to enhance code readability and maintainability by using more descriptive variable names throughout the project, and figured this would be an easy place to start.
For one example, the variable name cfg is used for configuration in some places. In our context, cfg could sometimes be interpreted as "classifier free guidance" which might cause confusion, so I'd propose config instead.
Another example is that the alias PathT is used in several places but PathType tab-completes just as easily in most editors and is easier to read. :)
If these kinds of minor improvements would be accepted, I'd be happy to create a tracker containing these and other examples and knock them out.

hello!
thanks for the PathT PR :)
Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

hello!
thanks for the PathT PR :)
Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

I looked through the code but couldn't find any use of the webui library except in `install.py'. so, it may be worth considering whether to use webui specifically or as a separate tool.
I believe that if you intend to keep using the name 'sd-webui' for this project, it would be advisable to modify the architecture so that it does not rely on the API and does not use Hydra. Otherwise If you only want to use webui's api, it would be better to develop it as an entirely separate tool.

this script works only with webui, through its api. Hence the webui in the name
the GUI is in the making, check out #28

Please note that I am not very good at English, so the meaning may not be complete.
I already know the "#28" you're referring to.
What I wanted to say is that using webui API does not make it an extension of webui. I consider it a separate tool that follows the structure of data in webui. However, as it appears that you have plans to implement it as GUI, I would like to suggest that it is better to develop using only the webui library, removing the API and Hydra from now on.
I hope you can provide a clear direction so that I can proceed with development accordingly. Otherwise, you may end up doing a lot of unnecessary work during development.

this script works only with webui, through its api. Hence the webui in the name
the GUI is in the making, check out #28

Please note that I am not very good at English, so the meaning may not be complete.
I already know the "https://github.com/s1dlx/sd-webui-bayesian-merger/pull/28" you're referring to.
What I wanted to say is that using webui API does not make it an extension of webui. I consider it a separate tool that follows the structure of data in webui. However, as it appears that you have plans to implement it as GUI, I would like to suggest that it is better to develop using only the webui library, removing the API and Hydra from now on.
I hope you can provide a clear direction so that I can proceed with development accordingly. Otherwise, I may end up doing a lot of unnecessary work during development.

Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

I guess it ultimately depends on your goals for the project. It attempts to build on BWM and address a sore need in the area of quickly finding an optimal mixture of 2 models. If it's intended to grow into something that continues to evolve to meet needs in this area, and is community maintained, then being very deliberate and clear with naming has some small advantages, I think. I suppose it's also a bit friendlier to users who may desire to peek under the hood for learning purposes.


Fix Message return types

Description
A number of Message methods have docblocks for @return Message which makes static analysis angry when a child class is returned. This PR switches to $this and specifies a self return type on the function. I also cleaned up the file a bit but nothing that would affect code.
Checklist:

[X] Securely signed commits
[X] Component(s) with PHPdocs
[ ] Unit testing, with >80% coverage
n/a User guide updated
[X] Conforms to style guide

Apparently the updated return types resolved this PHPStan error we were ignoring:

#Call to an undefined method CodeIgniter\HTTP\Message::setStatusCode()#

I removed that line and updated the file format to tabs (recommendation from https://ne-on.org) and alphabetized the sequences to be easier to find.

@samsonasik If you would please check over the PHPStan changes for me!

it seems ok 👍

I'm fine not finishing the tests again if you are.


Problems connecting due to user listing

After upgrading from 8e043ed->aae0fc9, I now have trouble connecting. It appears to just hang. I got:
time="2019-05-07T17:18:35+02:00" level=debug msg="trying login with personal token" prefix=matterclient
time="2019-05-07T17:21:11+02:00" level=error msg="login failedGet https://redacted&page=44&per_page=200: net/http: request canceled (Client.Timeout exceeded while awaiting headers)" module=matterircd

I left it for a while another time and it connected after 13 minutes.
I think the issue is loading all the users. Is there a way to architect this so it doesn't need to load users?

The latest commit now supports paging users, you must have a lot of users in a team?
I can add a setting so you can specify the amount of users to load in a channel.
But at the moment, the only solution is to go back to an older commit.

Which features require us to load all users preemptively?
The use-cases I can think of are:

/whois, for displaying all users on the server
&users channel

I don't really need either so I'm thinking those features could be turned off with a config option.
The alternative would be to only load users on-demand, when needed.

Also needed for the users in a #channel
If you have all those users in the same team, those users will also join the #town-square channel by default.
Querying 44 pages shouldn't take 13 minutes though, maybe some rate-limiting server side got hit?
I'll add an option to disable user loading.

Should be fixed in master, otherwise reopen please.


No search results since today

I've been using Youtube Mopidy quite happily for a few days, but since today it suddenly can't find results anymore.
The Mopidy log shows:
Apr 01 17:20:13 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube youtube LibraryProvider.search "{'artist': ['Da Tweekaz']}"
Apr 01 17:20:13 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube youtube LibraryProvider.search "{'album': ['Da Tweekaz']}"
Apr 01 17:20:13 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube youtube LibraryProvider.search "{'any': ['Da Tweekaz']}"
Apr 01 17:20:13 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube Searching YouTube for query "Da Tweekaz"
Apr 01 17:20:13 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube session.get triggered: search
Apr 01 17:20:14 raspberrypi mopidy[27062]: INFO     [YouTubeBackend-3] mopidy_youtube nothing in the soup, trying japi
Apr 01 17:20:14 raspberrypi mopidy[27062]: ERROR    [YouTubeBackend-3] mopidy_youtube No data found on page
Apr 01 17:20:14 raspberrypi mopidy[27062]: ERROR    [YouTubeBackend-3] mopidy_youtube search error "No data found on page"

I guess something changed on Youtube that requires mopidy-youtube to make some changes? youtube-dl is the latest version, at the moment 2021.4.1, but I believe that is not relevant for just search results.

Same issue here.
ápr 04 06:45:06 lz mopidy[62974]: INFO     2021-04-04 06:45:06,033 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   youtube LibraryProvider.search "{'any': ['metallica']}" ápr 04 06:45:06 lz mopidy[62974]: INFO     2021-04-04 06:45:06,033 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   Searching YouTube for query "metallica" ápr 04 06:45:06 lz mopidy[62974]: INFO     2021-04-04 06:45:06,033 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   session.get triggered: search ápr 04 06:45:06 lz mopidy[62974]: INFO     2021-04-04 06:45:06,034 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   session.get triggered: bs4api run_search ápr 04 06:45:06 lz mopidy[62974]: INFO     2021-04-04 06:45:06,725 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   nothing in the soup, trying japi ápr 04 06:45:06 lz mopidy[62974]: ERROR    2021-04-04 06:45:06,726 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   No data found on page ápr 04 06:45:06 lz mopidy[62974]: ERROR    2021-04-04 06:45:06,727 [62974:YouTubeBackend-3] mopidy_youtube ápr 04 06:45:06 lz mopidy[62974]:   search error "No data found on page" ápr 04 06:45:06 lz mopidy[62974]: WARNING  2021-04-04 06:45:06,728 [62974:Core-14] mopidy.core.library ápr 04 06:45:06 lz mopidy[62974]:   YouTubeBackend does not implement library.search() with "exact" support. Please upgrade it.
Mopidy version: 3.1.1
Mopidy-YouTube version: 3.2

I debugged my error. The search is not working, because when backend want to search on youtube, first you got a page that google using cookies ("Before you continue to YouTube").
After you agreed this, redirected the search page. This is the reason why bs4api cannot find anything.
I solved this problem with creating Google api key, and enabled api in the mopidy.conf for youtube plugin.

Good informations but i haven't api key, so i have updated "backend.py". Search for "cookie:" and change value to "PREF=hl=en; CONSENT=YES+cb.20210329-17-p0.fr+F+322;". I'have got consent value from my firefox session in developper mode or from extension cookie.txt. Now time to compile python3 -m py_compile  /usr/local/lib/python3.7/dist-packages/mopidy_youtube/backend.py for me. Reload and have fun with YT search

I don't have a key either, and I don't want one either. Thanks for the tip @christian3107, but according to my Firefox developer tools the CONSENT cookie is set to "PENDING+870" which I guess isn't good enough. Strange, as I can watch videos in my browser just fine without any cookie popups.

I'm not experiencing this issue, so hard for me to test.
Query (for @christian3107 ): if the consent cookie is set to CONSENT=YES; without including +cb.20210329-17-p0.fr+F+322 does it work?  I wouldn't be opposed to permanently setting CONSENT=YES; if that works for you, but I'd be reluctant to code the other stuff in - who knows what it means...?

Hi, same issue, it got past the no data found error after inputing the CONSENT with my specific cookie but then I get a new error:
May 09 18:08:27 livingroom mopidy[16285]: [youtube] azdwsXLmrHE: Downloading webpage
May 09 18:08:28 livingroom mopidy[16285]: ERROR    [ThreadPoolExecutor-2_0] mopidy_youtube audio_url error "'NoneType' object has no attribute 'get'"
May 09 18:08:28 livingroom mopidy[16285]: WARNING  [Core-7] mopidy.core.tracklist Track is not playable: youtube:video:azdwspxlmrhe
Any tips?

Hi, just tried with your cookie and the same happens.

Not sure where to go with this; I'll include a minimal cookie in the next version, and we'll see what happens.
In the meantime, the cookie is included in the yt-playlists branch, which will form the basis for the next release version.


typescript: send project name as argument for getSupportedCodeFixes

Blocks https://github.com/Microsoft/TypeScript/issues/28966
To enable TypeScript LanguageService plugins to intercept getSupportedCodeFixes, TypeScript needs to know which project the request belongs to, to forward to the correct LanguageService.
I don't know about the specific implementation details, but a parameter of type ts.server.protocol.FileRequestArgs should work.
Maybe VSCode also needs to call getSupportedCodeFixes for each project if it doesn't do that already.

Closing the VS Code side of this issue as there's nothing we can do on our side. If TS makes any changes to the server API, let me know and we'll adopt it


View-change not emitted on switching between day & week view

I don't receive view-change event when switching between week and day view.
If I switch from month view to day or week or the opposite, the event is emitted but not when switching between day and week.
It is the same in the events example calendar in the documentation: https://antoniandre.github.io/vue-cal/#ex--emitted-events

I think it is related to open issue #279

Hi @green-pigeon, thanks for reporting.
This should be fixed in version 3.2.5, documentation example works fine.


How to create a Bar Graph in pro motion screen?

Hi @manju-ms -- looks like I missed this. I would recommend joining Motioneers: http://motioneers.herokuapp.com.


sig-buildsystem: remove ownership for BUILD.bazel changes

What this PR does
Before this PR:
sig-buildsystem was added as reviewer and approver for changes to BUILD.bazel files.
After this PR:
sig-buildsystem is no longer added as reviewer and approver for changes to BUILD.bazel files.

Fixes #
Why we need it and why it was done in this way
Changes to BUILD.bazel files should be owned by approvers from the appropriate sig, since sig-buildsystem is not able to tell in all cases whether changes are applicable and valid.
The following tradeoffs were made:
The following alternatives were considered:
Links to places where the discussion took place:
Special notes for your reviewer
/cc @brianmcarey @xpivarc

Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

[ ] Design: A design document was considered and is present (link) or not required
[ ] PR: The PR description is expressive enough and will help future contributors
[ ] Code: Write code that humans can understand and Keep it simple
[ ] Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
[ ] Upgrade: Impact of this change on upgrade flows was considered and addressed if required
[ ] Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
[ ] Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
[ ] Community: Announcement to kubevirt-dev was considered

Release note

/release-note-none


Unable to change some options in the Other Tab

In the "Other tab" I'm unable to change walk speed or Enable/Disbale Item Collision
Thank You!

You need to update to the latest pre-release for those to work.


Add action APOLLO_MUTATION_ERROR

Adds a missed action that is dispatched when a mutation errors.

I think if we merge https://github.com/apollostack/apollo-client/pull/383, we will need to add mutations alongside this action?

My PR was 2 minutes earlier :P

Touche


Input Animation Recording is not working with Leap Motion

Describe the bug
When recording hand input using Input Simulation tools with Leap Motion, hand animation is not recorded at all. (Input recording works perfectly with HoloLens 2)
Only head movement is recorded.
To reproduce

Configure Leap Motion for hand tracking
Open the Input Simulation Tools window (Mixed Reality Toolkit > Utilities > Input SImulation)
Record the hand input
Playback shows only the head movement, not hand animation.

My setup

Tested in Unity version 2018.4.16f1, 2019.3.7f1, 2019.4.4f1
MRTK Version v2.4.0
Leap Motion mounted OpenVR headset

Target platform

OpenVR

Hi @hyungilkim I tested this out and it is currently working for me.
Steps:

Imported the MRTK 2.4 Foundation package into Unity 2019.4.5
Imported the Leap Core assets 4.5.0
Remained the Standalone platform
Opened a new scene
Added MRTK via Mixed Reality Toolkit > Add to Scene and Configure
Changed the configuration profile to the DefaultLeapMotionConfigurationProfile
Cloned the profiles and changed the Leap Controller Orientation to Desk

I also did hold my hand over the leap tracker prior to recording.

Thank you @CDiaz-MS for your help!
I tested with your steps, and it works.
(Setting Leap Controller Orientation to Desk in MRTK > Input > Input Data Providers > Leap Motion Device Manager)
However, when I change the Leap Controller Orientation to Headset, the recording did not work.

Hi @hyungilkim the fix in #8354 should have resolved this issue.  Please reopen this issue if the leap hands continue to not show up while using the input animation.

Hello @CDiaz-MS, the issue has resolved! thank you very much :)