Skip to content

Conversation

@aidenfoxivey
Copy link
Contributor

As per
GHSA-cpwx-vrp4-4pq7 use of Jinja 3.1.5 has a vulnerability. It's probably not an incredible problem, but it would be best to upgrade these packages to stay updated with bugfixes.

This follows the Nix flake model where the inputs are not pinned explicitly, so 'updating' them will actually increment their versions.

@coveralls
Copy link

coveralls commented Jul 24, 2025

Coverage Status

coverage: 82.752% (-0.003%) from 82.755%
when pulling 0efb289 on aidenfoxivey:dev
into 01de36c on open-quantum-safe:main.

@praveksharma
Copy link
Member

Thank you for getting to this @aidenfoxivey!

I ran a cursory search on requirements.txt/requirements.in and found that pip-compile (from pip-tools) translates the handwritten requirements.in to a version pinned requirements.txt. I am not certain that is the workflow that was user prior to this PR but as I understand it, the workflow in this PR is certainly different. Could you please explain the workflow for how a user would update a dependency once this PR is merged?

This follows the Nix flake model

Does this mean you're using nix to manage python dependencies instead of pip-tools?

Also, although /scripts/copy_from_upstream/requirements.* are useful for developers, these are not the files which track python dependencies in CI. I believe @h2parson looked into this recently; there might be other locations in .github/workflows and in the ci-containers repo. Hayden, could you please have a look and confirm if there are any other files which also track python dependencies?

@h2parson
Copy link
Contributor

I believe that when updating the requirements for SLH DSA I took the following steps (in addition to updating the files Pravek mentioned):

-Added the name of the pip requirement to .github/workflows/requirements.in

-Ran the command "pip-compile --generate-hashes --output-file=requirements.txt requirements.inpip-compile --generate-hashes --output-file=requirements.txt requirements.in" to populate .github/workflows/requirements.txt

-Then add the following line in the CI containers repo like so
https://github.com/open-quantum-safe/ci-containers/blob/530d6d247e4326c0ff4b3297612906ad9cfb27fa/ubuntu-latest/Dockerfile#L62

@aidenfoxivey
Copy link
Contributor Author

Could you please explain the workflow for how a user would update a dependency once this PR is merged?

A user can run pip-compile --upgrade to update the dependencies further.

Does this mean you're using nix to manage python dependencies instead of pip-tools?

Sorry, I was vague here. I mean that there is a lock file and a requirement file. The lock file is just "liboqs at commit foobarbaz uses this version of the python package".

My perspective is that unless the workflow depends on a specific version of a python package, the requirements.in file should not be pinned.

If you do pin requirements.in, this means that a user running pip-compile --upgrade will not actually get an updated requirements.txt, since the requirements file itself is pinned, instead of just the lock file.

@praveksharma
Copy link
Member

praveksharma commented Jul 28, 2025

Thanks for the explanation @aidenfoxivey, I think looks good to go if you're certain this the only place Jinja needs updating.

@aidenfoxivey
Copy link
Contributor Author

@praveksharma

~/s/liboqs % rg jinja
scripts/copy_from_upstream/copy_from_upstream.py
8:import jinja2
85:        contents = jinja2.Template(template).render(f)
90:        contents = jinja2.Template(template).render(f)
96:    contents = jinja2.Template(template).render({'instructions': instructions})
111:        contents = preamble + identifier_start + jinja2.Template(template).render(
126:    contents = preamble + identifier_start + jinja2.Template(template).render(f) + postamble

scripts/copy_from_upstream/requirements.txt
15:jinja2==3.1.6
24:    #   jinja2

From what I can tell this is the only use of Jinja in the repo!

# pip-compile
#
attrs==20.3.0 \
--hash=sha256:31b2eced602aa8423c2aea9c76a724617ed67cf9513173fd3a4f03e3a929c7e6 \
Copy link
Member

Choose a reason for hiding this comment

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

I think it is good practice to have the hashes of the packages here to reduce the risk of supply chain attacks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call - I just fixed it!

As per
GHSA-cpwx-vrp4-4pq7
use of Jinja 3.1.5 has a vulnerability. It's probably not an incredible problem,
but it would be best to upgrade these packages to stay updated with bugfixes.

This follows the Nix flake model where the inputs are not pinned explicitly, so
'updating' them will actually increment their versions.

Signed-off-by: Aiden Fox Ivey <[email protected]>
Copy link
Contributor Author

@aidenfoxivey aidenfoxivey left a comment

Choose a reason for hiding this comment

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

.

# pip-compile
#
attrs==20.3.0 \
--hash=sha256:31b2eced602aa8423c2aea9c76a724617ed67cf9513173fd3a4f03e3a929c7e6 \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call - I just fixed it!

@dstebila dstebila added the needs review Looking for a(nother) review label Aug 7, 2025
@dstebila dstebila removed the needs review Looking for a(nother) review label Aug 18, 2025
@dstebila
Copy link
Member

@aidenfoxivey We now have some merge conflicts here, are you able to resolve?

Signed-off-by: Aiden Fox Ivey <[email protected]>
@aidenfoxivey
Copy link
Contributor Author

@aidenfoxivey We now have some merge conflicts here, are you able to resolve?

Should be fixed.

@dstebila dstebila merged commit 1698d86 into open-quantum-safe:main Aug 20, 2025
98 of 102 checks passed
xuganyu96 pushed a commit to xuganyu96/liboqs that referenced this pull request Aug 26, 2025
* Upgrade Jinja to 3.1.6

As per
GHSA-cpwx-vrp4-4pq7
use of Jinja 3.1.5 has a vulnerability. It's probably not an incredible problem,
but it would be best to upgrade these packages to stay updated with bugfixes.

This follows the Nix flake model where the inputs are not pinned explicitly, so
'updating' them will actually increment their versions.

Signed-off-by: Aiden Fox Ivey <[email protected]>

* Fixed requirements merge conflict

Signed-off-by: Aiden Fox Ivey <[email protected]>

---------

Signed-off-by: Aiden Fox Ivey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants