Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Remove dependency on old Django admin internal templates (+ bump Django from 3.1 to >=3.2) #988

Closed
Jonasmadsen opened this issue Jun 12, 2022 · 6 comments
Labels
expected: next release size: hard status: backlog Work is planned someday but is not the highest priority at the moment status: done Work is completed and released (or scheduled to be released in the next version) touches: API/CLI/user interface touches: dependencies/packaging Issues or changes that add/remove/affect dependencies touches: js Pull requests that update Javascript code type: refactor why: security Intended to improve ArchiveBox security or data integrity
Milestone

Comments

@Jonasmadsen
Copy link

Currently archivebox uses django: "django>=3.1.3,<3.2" as defined in setup.py
However according to the django website:
https://www.djangoproject.com/download/
3.1.14 has been unsupported for quite a while.
This has some security implications and should be updated to django 3.2 (LTS) or newer.

I tried just updating the dependency to "django>=3.2,<3.3" but the test fails.
Unfortunately, there are breaking changes so it is not a simple version bump.

If anyone has a good idea of the scope of changes needed that would be great.
Love the project!

@pirate
Copy link
Member

pirate commented Jun 13, 2022

The changes are unfortunately complicated because I use some internal Django template components to build the UI (specifically the table-view action buttons), and those components were completely rewritten after 3.1.

@caj-larsson
Copy link

@pirate I know you are working on something of rewrite of much of the internals, do you have any plans on the template side of things? I decided to see how hard this would be and it seems to be no exaggeration that is quite a bit of work because of the usage of admin template/widgets.

Is it time to rebuild the foundation of the UI, if so got any thoughts on the direction?
If not, have you done any research on what needs to be done or suggestions to make this easier?

@pirate
Copy link
Member

pirate commented Nov 28, 2022

My inclination is to bridge the gap temporarily by including the old template files from the previous Django version's source code manually. I can create some template overrides that point to the old Django version for just the files needed, and default to all the newer files for everything else.

Then we can go through and rewrite those components/ remove the dependency on unstable Django internals entirely.

I knew this would be a pain eventually when I depended on those unstable templates but tbh they saved me a ton of dev time early on so I don't entirely regret incurring that tech debt.

In regards to the security fixes, last time I checked (2022/05) the two Django 3.1 CVEs were not hit by archivebox code paths, which is why I didn't push an urgent fix, however there may be newer CVEs since then that I haven't checked yet.

@pirate pirate changed the title Replace outdated Django version Remove dependency on old Django admin internal templates and upgrade Django from 3.1 to >=3.2 Jan 31, 2023
@pirate pirate added this to the v0.7.0 milestone Jan 31, 2023
@pirate pirate added size: hard why: security Intended to improve ArchiveBox security or data integrity touches: dependencies/packaging Issues or changes that add/remove/affect dependencies status: backlog Work is planned someday but is not the highest priority at the moment touches: API/CLI/user interface type: enhancement touches: js Pull requests that update Javascript code expected: delayed labels Jan 31, 2023
@pirate pirate changed the title Remove dependency on old Django admin internal templates and upgrade Django from 3.1 to >=3.2 Refactor: Remove dependency on old Django admin internal templates and upgrade Django from 3.1 to >=3.2 Jun 13, 2023
@pirate pirate changed the title Refactor: Remove dependency on old Django admin internal templates and upgrade Django from 3.1 to >=3.2 Refactor: Remove dependency on old Django admin internal templates (+ bump Django from 3.1 to >=3.2) Jun 13, 2023
@joepie91
Copy link

last time I checked (2022/05) the two Django 3.1 CVEs were not hit by archivebox code paths

For what it's worth, nixpkgs specifies a rather longer list of relevant CVEs:

Not all of these explicitly mark Django 3.1 as being affected; as I understand it, the reason they are added is because they appeared when 3.1 was already out of support, so it is likely that they also exist in 3.1 but nobody has ever explicitly confirmed that.

@pirate
Copy link
Member

pirate commented Oct 19, 2023

For now I have manually verified all of these CVEs, luckily ArchiveBox is not affected by any as we don't use the vulnerable code paths. Upgrading Django is still high on my priority list, but I'm not worried about security concerns unless other CVEs are announced that we are vulnerable to (and I have subscribed to notifications for this with Dependabot).

I also recommend everyone check out our new dedicated Security section on Github: https://github.com/ArchiveBox/ArchiveBox/security

You can subscribe to advisories and keep an eye our for new CVE announcements there too.

I understand this is causing trouble for our friends who help repackage archivebox for nix, arch, and other platforms that only offer newer Django releases. Sorry! Fixing this dependency is the next major internal work on my todo list.

@pirate
Copy link
Member

pirate commented Mar 26, 2024

Huge thanks @jimwins for doing this! #1388

@pirate pirate closed this as completed Mar 26, 2024
@pirate pirate added status: done Work is completed and released (or scheduled to be released in the next version) status: backlog Work is planned someday but is not the highest priority at the moment and removed status: backlog Work is planned someday but is not the highest priority at the moment labels Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expected: next release size: hard status: backlog Work is planned someday but is not the highest priority at the moment status: done Work is completed and released (or scheduled to be released in the next version) touches: API/CLI/user interface touches: dependencies/packaging Issues or changes that add/remove/affect dependencies touches: js Pull requests that update Javascript code type: refactor why: security Intended to improve ArchiveBox security or data integrity
Projects
None yet
Development

No branches or pull requests

4 participants