fix: git safe repo directory for docker image

[liketechnik]

Fixes an issue introduced with a recent git update
(https://github.blog/2022-04-12-git-security-vulnerability-announced/)
with a common workaround (actions/checkout#762,
https://stackoverflow.com/questions/71901632/fatal-error-unsafe-repository-home-repon-is-owned-by-someone-else,
actions/checkout#760),
by marking the /data directory inside the container as safe for git
during the container build.

Fixes #17

Depends on #19

[liketechnik]

@malt-r Weil ich das Problem nicht reproduziert kriege, wäre es super, wenn du einmal ausprobieren könntest, ob die Änderung das Problem behebt.

Du kannst dir mit bash -c 'wget https://github.com/Compilerbau/CB-Lecture-Bachelor/pull/16.diff && git apply 16.diff && rm 16.diff' die Änderungen in deinen lokalen Branch holen (musst halt aufpassen die .github/actions/alpine-pandoc-hugo/Dockerfile dann nicht aus versehen bei dir zu committen); dann kannst du das Skript direkt für die entsprechenden Branches ausführen.

Noch wichtig: Leider muss zum Ausprobieren einmal das Docker-Image neu gebaut werden.

[cagix]

gab es probleme? könnt ihr bitte noch ein issue dazu aufmachen und die probleme kurz beschreiben und dann den pr verlinken?

sonst ist das irgendwie seltsam :) ... ein fix für ein problem, was ich gar nich habe/kenne ...

[liketechnik]

@cagix: Das Problem ist jetzt in #17 beschrieben und der entsprechende Issue in die Beschreibung von dieser PR aufgenommen.

[cagix]

@cagix: Das Problem ist jetzt in #17 beschrieben und der entsprechende Issue in die Beschreibung von dieser PR aufgenommen.

verstehe. hätte nicht gedacht, dass wir das damit triggern? ist der user im docker-container ein anderer als der auf der konsole? das wäre imho der bessere fix? alternativ könnte man die komplette aktion auch im docker laufen lassen (checkout, änderungen, commit, push).

irgendwie fühlt sich das falsch an - genauso wie die permanente abfrage von vsc, ob ich einem ordner vertraue. ich meine, man kann das doch eh nie wirklich mit letzter gewissheit sagen und setzt deshalb immer alles auf "ja, bitte machen". was hat man also gewonnen (außer mehr arbeit)?

[liketechnik]

Der Nutzer im Container kann ein anderer sein, als der auf der Konsole. Das kann u. a. verursacht sein weil:

• im 'klassischen' Docker-setup der Container als root-Nutzer läuft - wird aber eigentlich durch die -u Flag im Makefile umgangen
• der Container in einem separaten Linux-Namespace läuft mit anderen Nutzer/Gruppen-Ids (z. B. Docker mit User Namespaces, Podman)

Podman bietet einen Switch an (--userns=keep-id), um die Ids im Namespace denen des Host anzupassen. Das ist nach meiner Erfahrung in diesem Repo generell nötig, da z. B. hugo und pandoc in das gemountete Dateisystem schreiben. Dieser funktioniert auch, um das vorliegende Problem zu lösen, ist aber halt nicht mehr Docker, sondern podman, sodass das auch nicht wirklich die generelle Lösung sein kann.

Das Docker äquivalent --userns=host hat im Test mit @malt-r leider keine Besserung gebracht.

Fazit

Ja, diese PR ist nur ein Workaround, kein 'richtiger' Fix des eigentlichen Problems, dass der Nutzer im Container nicht mit dem Nutzer außerhalb übereinstimmt(/übereinstimmen kann).

[liketechnik]

(Allerdings: Wenn ich die Gefahr, die die Fehlermeldung von Git auslöst, richtig verstanden habe, sorgt der Container schon dafür, dass diese nicht mehr besteht: Das oberste .git-Verzeichnis innerhalb des Containers kann nur das Repo selber, und keines auf höherer Ebene dass einem anderen Nutzer gehört, sein. Schließlich ist nur der Ordner selber im Container sichtbar, und nicht der darüberliegende Ordner aus dem Host-System. Insofern sollte das ignorieren dieses Fehlers/Warnung innerhalb des Containers nicht sicherheitsrelevant sein.)

[malt-r]

Aktuell besteht bei mir das Problem, dass name und email von git im container nicht konfiguriert sind, und sich git daher weigert, die commits zu machen. Die REM-Tags werden allerdings trotzdem gelöscht (wahrscheinlich interessant für dich @liketechnik ). Testweise habe ich im Dockerfile RUN git config --global user.name ... eingefügt, dann funktioniert das Skript. Muss ich da noch irgendwas beachten, damit die Konfiguration in den container übernommen wird?

[liketechnik]

Nein. Die Dockerfile ist schlecht getestet. Da ich mit unterschiedlichen E-Mails für FH/anderen Kram arbeite ist das Zeug bei mir für jedes Repo einzeln konfiguriert. Deswegen tut das auch wenn ich das teste. Ich würde dafür aber ein neues Issue aufmachen.

Aber das der Fix an sich funktioniert ist ja schonmal schön zu hören.

[cagix]

Uff. Die Git-Konfig sollte nicht ins Docker-File, das soll ja unabhängig vom konkreten User funktionieren. Da müssen wir uns was anderes einfallen lassen.

Edit I: Also temporär für diese eine Aktion ist das schon okay, aber dann die Änderungen am Dockerfile bitte nicht pushen.

Edit II: Irgendwo habe ich im Zusammenhang mit VSCode und dem remote-Arbeiten im Docker-Container mal irgendwas gesehen Richtung Git-Config. Finde den Link aber grad nicht.

[liketechnik]

Was ich unschön finde: Den Ordner /data definieren wir an anderer Stelle (Makefile). D.h. wir führen damit zwei Stellen ein, die synchron gehalten werden müssen. Ich sehe aber tatsächlich auch keine andere Lösung.

Eine schönere Lösung ist denke ich die GIT_DIR Variable für die Ausführung vom Script auf /data/.git zu setzen. Vorteil: der Ort vom Repo im Container ist dann einheitlich nur in der Makefile.

[liketechnik]

Ich habe das mal entsprechend umgesetzt und getestet. Wenn ich ohne diesen PR die entsprechende Fehlermeldung bekomme löst auch der neue Commit die Fehlermeldung.

[liketechnik]

Ich habe den PR jetzt bei mir lokal getestet. Wenn nicht noch weitere Unterschiede zwischen verschiedensten Git und/oder Docker-Konfigurationen für Probleme sorgen, die ich nicht auf dem Schirm habe, funktioniert das in der Form.

[cagix]

@liketechnik Zwei Fragen:

1. Die Argumente für die Docker-Parameter sind sonst immer in Anführungsstrichen verpackt. Muss/sollte das bei DOCKER_GIT_ENV auch so sein?
2. Der Mount-Point "/data" taucht an mind. drei Stellen auf (Z. 36, Z. 37, Z. 47, vielleicht noch an weiteren Stellen). Ich würde dem Ding eine Variable spendieren, dann kannst Du auch den Kommentar in Z. 46 wieder entfernen.

[cagix]

ein "/data" war noch (siehe vorschlag).

da das auch #19 betrifft: könnten wir DOCKER_REPO_MNTPOINT in DOCKER_MNTPOINT umbenennen? die variable ist recht lang, und das "repo" ist für mich nicht so richtig signifikant ...

[cagix]

Das sollte (hoffentlich) die Backslashes bündig untereinander schreiben ... :)

Im Prinzip scheint dann alles zu passen, ich kann zur Not auch mit "DOCKER_REPO_MNTPOINT" leben.

Mir ging grad noch ein Gedanke durch den Kopf: Wie reagiert unser GH-Workflow auf diese Änderungen? Sind die Variablen "user.name" und "user.email" im Workflow überhaupt gesetzt? Was passiert, wenn die leer sind?

[liketechnik]

Die unpassende Einrückung in der ersten Zeile liegt an der Tab-Weite der Darstellung, weil die nachfolgenden Zeilen per Tab eingerückt sind. Soll ich das noch anpassen?

Bzgl möglicher Probleme des Workflows: Die Änderung wirkt sich nur auf beim Aufruf des delete-rem-tags Targets/Skripts. Das wird im Workflow soweit ich weiß eh nicht aufgerufen. Ansonsten: Ich weiß nicht ob/was in der Ausführungsumgebung der Workflows für user.name/user.email gesetzt ist. Das sollte aber ebenfalls egal sein, da im Zweifel git halt einen leeren String zurück gibt und der dann entsprechend weitergegeben wird. Aber der wäre ja sonst auch leer.

[cagix]

Die unpassende Einrückung in der ersten Zeile liegt an der Tab-Weite der Darstellung, weil die nachfolgenden Zeilen per Tab eingerückt sind. Soll ich das noch anpassen?

ja, mach mal. sieht schöner aus, wenn das hinten in einer spalte steht ...

Bzgl möglicher Probleme des Workflows: Die Änderung wirkt sich nur auf beim Aufruf des delete-rem-tags Targets/Skripts. Das wird im Workflow soweit ich weiß eh nicht aufgerufen.

stimmt. insofern ist die frage irrelevant :)

Ansonsten: Ich weiß nicht ob/was in der Ausführungsumgebung der Workflows für user.name/user.email gesetzt ist. Das sollte aber ebenfalls egal sein, da im Zweifel git halt einen leeren String zurück gibt und der dann entsprechend weitergegeben wird. Aber der wäre ja sonst auch leer.

ja.

---

ich hab grad noch ne andere (evtl. dumme) idee. im moment baue ich jedes mal im workflow ein neues docker-image und lasse die sachen dann in diesem container laufen. jonas hatte schon mal experimentiert, ob man zeit gewinnen würde, wenn man das image in der gh-registry ablegt und im workflow nur zieht - aber das hat nicht wirklich was gebracht (sagte er damals). wenn man die installationsskripte für die debian-pakete, die man beim erstellen des docker-images ausführt, direkt im ubuntu-runner ausführen würde und danach direkt im ubuntu-runner arbeiten würde, müsste das am ende doch schneller sein, oder? man spart sich das ziehen des basis-images und den overhead des dockerstarts für jeden befehl. was denkst du? lohnt es sich, hierfür nochmal ein ticket aufzumachen und da weiter zu experimentieren? oder ist das nur ne spinnerte idee, die am ende nicht viel bringt? oder sollte man eher nochmal der registry-sache nachgehen (eigentlich müsste das doch deutlich schneller sein als das image neu zu erstellen?!).

[liketechnik]

So, die Makefile Formatierung passt jetzt.

---

Wegen dem Container würde ich auf jeden Fall ein neues Issue aufmachen. Das kann ich gerne erledigen, dann packe ich da auch gleiche meine Gedanken zu mit rein (wenn ich sie mir gemacht habe :P)

[cagix]

lgtm

[cagix]

@liketechnik tysen tak!