Skip to content

audit: Open Source Readiness — fix credentials, generalize config, full checklist - #11

Closed
SpiderQubit with Copilot wants to merge 2 commits into
masterfrom
copilot/review-tf-code-for-openness
Closed

audit: Open Source Readiness — fix credentials, generalize config, full checklist#11
SpiderQubit with Copilot wants to merge 2 commits into
masterfrom
copilot/review-tf-code-for-openness

Conversation

Copilot AI commented Apr 28, 2026

Copy link
Copy Markdown

Overview

This PR prepares the repository to be made public. It fixes the most critical issues immediately and documents everything else in a new OPEN_SOURCE_CHECKLIST.md that can serve as a working issue tracker.


Changes in this PR

🔐 Credentials & secrets fixed

File What changed
tf/settings-template.py lucash@fastmail.comadmin@example.com
docker-compose-template.yml Hardcoded MariaDB passwords replaced with <CHANGE_ME_…>
docker-compose-template.yml it@tekniskfysik.seyour-email@example.com
docker-compose-template.yml rt.tekniskfysik.seyour.domain.example.com
docker-compose-template.yml Absolute paths (/root/rodatraden/…) → relative paths (.)

📝 Documentation cleaned up

  • DOCKER_README.md — added a prominent note that the file describes a specific internal deployment; paths and service names must be adapted
  • README.md — removed duplicate #### Windows heading
  • tf/settings-template.py — removed pinned Django 2.2.1 version from module docstring

✅ New: OPEN_SOURCE_CHECKLIST.md

A comprehensive list of everything else to address before going public. See the file for full detail; summary below.


Remaining issues (tracked in OPEN_SOURCE_CHECKLIST.md)

🔐 Security

Font Awesome kit ID exposed in base.html:

<!-- rodatraden/templates/rodatraden/base.html, line 88 -->
<script src="https://kit.fontawesome.com/c6e9b99eeb.js"></script>

This kit ID is tied to a specific Font Awesome account. Anyone forking the project uses your quota. Consider self-hosting Font Awesome or documenting that each deployer must create their own kit.


🏷️ TF-specific naming

tf/ directory name: The entire Django project configuration lives in a folder called tf (Teknisk Fysik). This name leaks through into settings.py, wsgi.py, manage.py, the Dockerfile, and deployment docs. Renaming it (e.g. to config/) is a clean but mechanical refactor:

# manage.py (and tf/wsgi.py) currently:
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'tf.settings')

# After rename:
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'config.settings')

Hardcoded programme description on the homepage (index.html, line 19):

Röda tråden är en databas med information om kurserna på Teknisk fysik i Umeå.

And in profile_list.html (line 24):

Som student på Teknisk Fysik i Umeå har du, utöver möjligheten…

These could be moved to a SITE_PROGRAMME_NAME / SITE_DESCRIPTION setting so any university can deploy without editing templates.

Hardcoded UMU video link (block_detail.html, line 90):

<a href="http://www.tp.umu.se/tekniskfysik/Instruktionsvideo_R%C3%B6daTr%C3%A5den_v1.mp4">

This is a dead (or soon-to-be-dead) internal UMU link. Make it configurable or remove it.

Exam model docstring (models.py, line 753):

class Exam(models.Model):
    """Exams such as teknisk fysik."""

Should be generic, e.g. """Degree/exam definitions.""".


🛠️ Setup from scratch

Migrations not committed (.gitignore excludes rodatraden/migrations/0*_*.py):
A fresh git clone has no migration files, so the correct setup sequence is:

python manage.py makemigrations rodatraden
python manage.py migrate

This is already mentioned in the README but could easily be missed. Consider committing the initial migrations (standard Django practice for apps distributed as a project).

No initial data fixture — and hard-coded default=1 on FK fields:

# rodatraden/models.py
department = models.ForeignKey(Department, ..., default=1)
level      = models.ForeignKey(Level,      ..., default=1)

After a fresh migrate, the database is empty. Trying to create a course immediately will crash with an IntegrityError because Department.id=1 and Level.id=1 don't exist yet. The README already lists what needs to be created manually in the admin panel (Institutioner, Nivåer, Tidsperioder, …), but a seed fixture would make the experience far smoother:

# Export after creating the required objects:
python manage.py dumpdata rodatraden.Department rodatraden.Level \
    rodatraden.TimePeriod --indent 2 -o rodatraden/fixtures/initial_data.json

# Load on a fresh install:
python manage.py loaddata initial_data

requirements.txt is unpinned:

django  # Base django module
Pillow  # Not sure to be frank

Unpinned deps can break silently when a new major version is released. A requirements-lock.txt (from pip freeze) should be used for production installs.


🐛 Code quality / TODO annotations

urls.py, line 135 — "can't be arsed":

# TODO: The <int:pk> is only to avoid complaints by the generic view. This
# should be removed, but I can't be arsed right now. Double security perhaps?
path('anvandare/<str:username>/<int:pk>/andra', views.UserUpdate.as_view(), ...),

The informal tone is fine internally but not suitable for a public repo. Either fix the URL pattern or replace the comment with a proper explanation.

views.py, line 1726 — enable=0 treated as truthy:

# XXX: Will also be true for "enable=0" or "enable=false" etc.
if (shouldEnable):
    block.should_verify_prerequisites = True

Suggested fix:

block.should_verify_prerequisites = shouldEnable.lower() in ('1', 'true', 'yes')

views.py, line 1379 — typo prinvate_courses_json:

prinvate_courses_json = [course.as_json() for course in block.privatecourses.all()]

profile_detail.html — hardcoded course list marked "first draft":

{# Hardcoded for now, without links. This is a first draft. #}
<li>Forsknings- och utvecklingsprojekt inom teknisk fysik (Lp1,2,3,4, Sommar)</li>

This section is TF-specific and was never completed. Remove or make it database-driven.


🚀 Nice-to-have

  • Add a CONTRIBUTING.md with a single-page "getting started" guide
  • Add a LICENSE file (none present)
  • Add a .env.example listing all environment variables
  • Run python manage.py check --deploy and fix any warnings

Existing open PRs

There are currently several open PRs targeting master:

Most of the issues above exist in the current master branch. Once those PRs are merged, the same issues may need to be rechecked (especially the new template content introduced in #7).

…ss checklist

Agent-Logs-Url: https://github.com/it-amanuens/rodatraden/sessions/2e8fc556-e14e-49bf-b3fa-161d46d0a43d

Co-authored-by: SpiderQubit <25958778+SpiderQubit@users.noreply.github.com>
Already updated in master
@it-amanuens

Copy link
Copy Markdown
Owner

I am not sure if this deleted docker compose files is the correct way to go.

Comment thread tf/settings-template.py
SpiderQubit pushed a commit that referenced this pull request Apr 30, 2026
…ult email (suggestions from #11)

Co-authored-by: Copilot <copilot@github.com>
@SpiderQubit
SpiderQubit deleted the copilot/review-tf-code-for-openness branch May 1, 2026 15:21
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.

3 participants