Skip to content

Lisatud link lehele leksiko.eu - #3

Merged
liikane merged 1 commit into
mainfrom
dev-homepage
May 13, 2026
Merged

Lisatud link lehele leksiko.eu#3
liikane merged 1 commit into
mainfrom
dev-homepage

Conversation

@liikane

@liikane liikane commented May 13, 2026

Copy link
Copy Markdown
Owner

No description provided.

@liikane
liikane merged commit 6190618 into main May 13, 2026
1 check passed
@liikane
liikane deleted the dev-homepage branch May 13, 2026 11:41
@devin-ai-integration

Copy link
Copy Markdown

Code review — liikane/tallinn

Scope

You have only one PR authored by you in this repo: #3 "Lisatud link lehele leksiko.eu".
PR #2 ("Remove broken tallinn submodule gitlink") was authored by the Copilot SWE agent bot, so per your instruction it is excluded.

To cover the requested depth I reviewed PR #3 plus your 5 most recent non-merge commits:

# Commit Subject
1 d935376 Create SECURITY.md for security policy
2 886240c Free PHP zipper and unzipper (zipper-unzipper.zip)
3 3ae6ffa Add editor notes to kana.txt
4 939b33a Masinast githubi (kana.txt)
5 cb6219f Add additional lines to kana.txt

Severity legend: [HIGH] fix soon · [MED] should fix · [LOW] nit.


Headline findings

  1. [HIGH] zipper-unzipper.zip is a public, unauthenticated remote file-write tool served from https://leksiko.eu/zipper-unzipper.zip (HTTP 200). It is functionally a webshell-grade utility: no auth, no CSRF token, path traversal in both input fields. Hosting it on your production domain also risks Safe Browsing / AV flagging of the domain.
  2. [HIGH] Two different phone numbers on the live site — hero says +372 555 15 285, contact block says +372 555 51 285. One is wrong, and it's the primary "urgent help" CTA.
  3. [MED] SECURITY.md is the unmodified GitHub template — placeholder instructions and a fake version table (5.1.x / 4.0.x) for a repo that has no versions. It tells a reporter nothing.
  4. [MED] The Add Issue to Project workflow fails on every run (both runs on 2026-08-18 failed at startup). Almost certainly secrets.GITHUB_TOKEN — it has no project scope, so actions/add-to-project cannot write to a user project; a fine-grained PAT is required.
  5. [MED] Scratch files are publishedkana.txt and test.txt both return 200 on the live domain.

PR #3 — "Lisatud link lehele leksiko.eu"

Diff: README.md (-13/+3), SUMMARY.md (title change). Correct and low-risk; a few things I'd change.

  • [MED] Content deletion, not relocation. The whole "WP uudiste lisamise juhend" checklist was removed with no pointer to where it went. If it moved to liikane/wp-juhendid, the new README should link there; otherwise the only copy is git history, where nobody will look.
  • [LOW] Task-list syntax used for a non-task. * [ ] Kontaktid ja muu info on [siin lehel](...) renders as an unchecked checkbox, implying pending work. A plain bullet (or a sentence) is what you want here.
  • [LOW] Link text is not descriptive. "siin lehel" is the classic anti-pattern for screen readers and for GitBook search. Prefer [leksiko.eu](https://leksiko.eu) or "Kontaktid ja muu info: leksiko.eu".
  • [LOW] SUMMARY.md title grammar. "Kontaktid info" isn't Estonian — either Kontaktinfo or Kontaktid. Also, SUMMARY.md title and the README.md H1 ("Täpsem info") now disagree; GitBook shows the SUMMARY label, so the two names drift.
  • [LOW] Missing-newline churn. The old file had no trailing newline; the fix is good, but it's the kind of thing an .editorconfig (insert_final_newline = true) would keep consistent — kana.txt and test.txt still lack one.
  • [LOW] Empty PR description. For a one-line change it's survivable, but the PR that changed the repo's front door says nothing about why.

Positive: the change is scoped, the external link uses HTTPS, and the title attribute is genuinely useful context.


Commit 1 — d935376 "Create SECURITY.md for security policy"

  • [MED] Pure template, zero information. Both sections still say "Use this section to tell people…". A reporter learns no contact address, no response time, no scope.
  • [MED] The version table is fiction. This repo is a single static page with no releases; claiming 5.1.x supported / 5.0.x unsupported is actively misleading. Delete the table or replace it with "Only the current published version of leksiko.eu is supported."
  • [LOW] No reporting channel. Give an email (e.g. the address already on the site) or, better, enable GitHub Private vulnerability reporting (Settings → Code security) and point at it. Note the site publishes a mailto-style address as plain text — if you list it in SECURITY.md too, expect scraping; a dedicated security@ alias is cleaner.
  • [LOW] Scope mismatch. The repo also distributes third-party GPL PHP code (zipper-unzipper.zip). A security policy that doesn't say whether vulnerabilities in that bundled tool are in scope is a gap — see below.

Suggested replacement (Estonian or English, whichever the audience is):

# Turvapoliitika

Toetatud on ainult leksiko.eu praegune avaldatud versioon (haru `main`).

## Haavatavusest teatamine
Teata privaatselt GitHubi "Report a vulnerability" kaudu või kirjuta <security@...>.
Vastan 5 tööpäeva jooksul. Palun ära ava avalikku issue't.

Commit 2 — 886240c "Free PHP zipper and unzipper"

This is the commit I'd revert first. It adds a 3.7 KB binary zip containing zipper-unzipper.php (422 lines, attec.at Unzipper 0.1.1, GPLv3).

Distribution / repo-hygiene problems

  • [HIGH] It is publicly downloadable from your production domain. curl -o /dev/null -w %{http_code} https://leksiko.eu/zipper-unzipper.zip200. Anyone (and any crawler/AV vendor) can pull a drop-in remote-file-write tool from leksiko.eu. If it's meant as a resource for readers, host it behind the site's normal download flow with a warning, or link upstream instead of re-hosting.
  • [MED] Binary blob in git. A zip can't be diffed or reviewed, and every future version bloats history. Commit the .php source (reviewable, greppable) and generate the zip at release time if you need one.
  • [MED] GPLv3 redistribution without license text or attribution. The repo has no LICENSE, and nothing outside the zip credits Andreas Tasch. GPLv3 §4 wants the license and notices to travel with the code. Add LICENSE (or a THIRD_PARTY.md) naming the upstream project, version, and license.
  • [LOW] Pinned to an old upstream (0.1.1). No note on provenance or how to update, so nobody will ever know it's stale.

Bugs and vulnerabilities in the bundled PHP

Reviewed because you're the one shipping it; if you keep it, ship it patched or with a loud warning.

  • [HIGH] No authentication, no CSRF token. Any HTTP POST — including a form auto-submitted from an attacker's page in an authenticated admin's browser — triggers extraction or archive creation (if (isset($_POST['dounzip'])), dozip). The upstream README's "delete after use" is the only control, and it's not enforced.
  • [HIGH] Path traversal in extpath. $destination is only strip_tags()-ed, then concatenated: $extpath = $this->localdir . '/' . $destination. ../../ escapes the directory, mkdir() creates it, and the archive is extracted outside the intended tree. Extracting a zip of .php files into a webroot is RCE.
  • [HIGH] Path traversal + information disclosure in zippath. Zipper::zipDir('..', 'zipper-<date>.zip') archives the parent tree (wp-config.php, .env, DB credentials) into the web-served directory under a fully predictable name (zipper-Y-m-d--H-i.zip), so an unauthenticated attacker can trigger it and then just download it.
  • [MED] Zip-slip via the tar path. ZipArchive::extractTo() rejects ../ entries in modern libzip, but PharData::extractTo() on the intermediate .tar (extractGzipFile) has no equivalent guarantee, and the .gz branch writes $destination . '/' . $filename with no containment check at all.
  • [MED] XSS. echo "<option>$zip</option>" interpolates an on-disk filename unescaped, and reset($GLOBALS['status']) prints messages containing raw HTML (extractRarArchive deliberately emits an <a>). strip_tags() on input is not an output-encoding strategy — use htmlspecialchars() at every echo.
  • [MED] Unchecked I/O. gzopen()/fopen() results are used without checks (fwrite(false, …)), mkdir()'s return value is ignored (and it's non-recursive, so a nested extpath silently fails), and ZipArchive::open(..., CREATE) in zipDir is never checked before addEmptyDir.
  • [LOW] while ($string = gzread($gzipped, 4096)) terminates on any falsy chunk (e.g. a chunk that is exactly "0") instead of on false, truncating output. Use while (!gzeof(...)) or an explicit === false check.
  • [LOW] PHP 8 deprecation. With no action taken, $GLOBALS['status'] from the constructor is always set, but strtoupper(key(...)) on an empty array passes null → deprecation notice on 8.1+ and a bogus status-- class.
  • [LOW] Self-inclusion race in folderToZip. It skips only basename(__FILE__), so the archive currently being written (and every other pre-existing .zip) is added to itself — bloated and potentially corrupt output.
  • [LOW] A11y. <label for="zipfile"> / for="extpath" point at controls that have name but no id, so the labels are unbound.

Recommendation: git rm the zip from main (history rewrite isn't worth it), and if you want to offer the tool, link upstream with a one-paragraph warning: unauthenticated, delete immediately after use, never leave it on a live server.


Commits 3–5 — 3ae6ffa, 939b33a, cb6219f (kana.txt)

These are editor-scratch commits ("VS Code Toimib", "Proovime ka NPP", "NPP on palju kiirem kui VS Code") on a repo that is the deploy source for leksiko.eu.

  • [MED] Test files are published. https://leksiko.eu/kana.txt and /test.txt both return 200. Harmless content today, but it's unintended public surface and it tells visitors the site root is a scratchpad. Move them out of the deploy root, delete them, or (if you must keep them) add an _exclude-style layout — with GitHub Pages the simplest fix is deleting them and testing sync in a throwaway repo.
  • [LOW] Commit messages don't describe changes. "Masinast githubi" and "Add additional lines to kana.txt" are indistinguishable from each other; three commits over one day all add lines to the same scratch file. Squash-style discipline (or just not committing scratch files) removes the noise.
  • [LOW] No trailing newlines in kana.txt / test.txt — every future edit shows a spurious "\ No newline at end of file". An .editorconfig fixes this class of churn permanently.
  • [LOW] Repeated merge commits (d7d312e, 99b992c, "Kokku pandud veebis ja masinas tehtud muudatused") come from editing both on github.com and locally. git pull --rebase (or git config --global pull.rebase true) keeps the history linear.

Repo-wide issues worth fixing (found while reviewing the above)

These are in index.html / CI rather than in the five commits, but they affect the live site.

  • [HIGH] Conflicting phone numbers+372 555 15 285 (hero card) vs +372 555 51 285 (contact lead). Pick one.
  • [MED] Mobile navigation is unreachable. @media (max-width: 900px) { .nav-links { display: none; } } hides the nav with no hamburger replacement, so phone visitors get no Teenused/Tööd/Kontakt links (the in-page CTAs partially compensate).
  • [MED] Add Issue to Project workflow fails every run. Runs 32149346725 and 32148547036 both failed. secrets.GITHUB_TOKEN cannot write to a user-level project — supply a fine-grained PAT with project: write as e.g. secrets.ADD_TO_PROJECT_PAT. Also add an explicit permissions: block and pin the action (actions/add-to-project@v1.0.2; upstream is already at v2.0.0, so @v1 is behind).
  • [LOW] Email/domain mismatch worth double-checking: the site asks people to write to lauri@leksiko.edu.ee while the site itself is leksiko.eu (CNAME). If both domains are yours this is fine, but it reads like a typo to visitors.
  • [LOW] </br> is not a valid tag (contact-lead paragraph) — use <br />.
  • [LOW] Missing rel="noopener noreferrer" on three target="_blank" links (wp-juhendid, homepage, klaaskajak.ee, ubari.ee) — the Eurotermbank/Keeleveeb ones have it, so it's just inconsistency.
  • [LOW] Contact form UX: the submit button isn't disabled during fetch, so double-clicking sends duplicate Formspree submissions; there's no honeypot/_gotcha field, so expect bot spam.
  • [LOW] No favicon, no og:/twitter: meta, no robots.txt/sitemap on a site whose selling point is SEO consulting.
  • [LOW] Hardcoded © 2026 in the footer.

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.

1 participant