Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions pontoon/sync/core/checkout.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging

from os import walk
from os.path import join, normpath, relpath
from os import sep, walk
from os.path import join, normpath, realpath, relpath
from typing import NamedTuple

from pontoon.base.models import Project, Repository
Expand All @@ -11,6 +11,18 @@
log = logging.getLogger(__name__)


def is_inside(root: str, path: str) -> bool:
"""
Is `path` inside `root`, once the two have been fully resolved?

Comparing paths as strings is not enough, because a path that looks
contained may still resolve to a file elsewhere on the filesystem.
"""
root = realpath(root)
path = realpath(path)
return path == root or path.startswith(root + sep)
Comment thread
mathjazz marked this conversation as resolved.
Outdated


class Checkout:
repo: Repository
is_source: bool
Expand Down Expand Up @@ -77,6 +89,15 @@ def __init__(
self.removed = delta[1] if delta else []
self.renamed = []

# A repo can commit a symlink pointing anywhere on the filesystem.
inside: list[str] = []
for co_path in self.changed:
if is_inside(self.path, join(self.path, co_path)):
inside.append(co_path)
else:
log.error(f"[{slug}:{co_path}] Skipping path outside the checkout")
self.changed = inside


class Checkouts(NamedTuple):
source: Checkout
Expand Down
9 changes: 8 additions & 1 deletion pontoon/sync/core/translations_to_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
User,
)
from pontoon.base.models.changed_entity_locale import ChangedEntityLocale
from pontoon.sync.core.checkout import Checkouts
from pontoon.sync.core.checkout import Checkouts, is_inside
from pontoon.sync.repositories import CommitToRepositoryException, get_repo


Expand Down Expand Up @@ -247,6 +247,13 @@ def update_changed_resources(
try:
lc_plurals = locale.cldr_plurals_list()
tr_res = build_translated_resource(locale, lc_translations, res)
# A symlinked target should not redirect the write out of the checkout
if paths.base and not is_inside(paths.base, target_path):
log.error(
f"[{project.slug}:{path}, {locale.code}] "
"Resource path outside the checkout"
)
continue
makedirs(dirname(target_path), exist_ok=True)
with open(target_path, "w", encoding="utf-8") as file:
for line in serialize_resource(tr_res, gettext_plurals=lc_plurals):
Expand Down
34 changes: 34 additions & 0 deletions pontoon/sync/tests/test_checkouts.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from os import symlink
from os.path import join
from tempfile import TemporaryDirectory
from typing import Any
from unittest.mock import Mock, patch
Expand Down Expand Up @@ -115,6 +117,38 @@ def test_no_changes_with_no_prev_commit():
]


def test_changed_excludes_paths_outside_checkout():
"""A repo can commit a symlink pointing anywhere, so paths that resolve
outside the checkout are dropped. Links that stay inside are kept."""
tree: FileTree = {"checkout": {"en-US": {"ok.ftl": ""}}, "elsewhere.ftl": ""}
with TemporaryDirectory() as root:
build_file_tree(root, tree)
co_path = join(root, "checkout")
symlink(join(root, "elsewhere.ftl"), join(co_path, "en-US", "escapes.ftl"))
symlink(join(co_path, "en-US", "ok.ftl"), join(co_path, "en-US", "inside.ftl"))

mock_vcs = MockVersionControl(
changed=[
join("en-US", "ok.ftl"),
join("en-US", "escapes.ftl"),
join("en-US", "inside.ftl"),
]
)
mock_repo = Mock(
Repository,
branch="BRANCH",
checkout_path=co_path,
last_synced_revision="def456",
source_repo=True,
url="URL",
type=Repository.Type.GIT,
)
with patch("pontoon.sync.core.checkout.get_repo", return_value=mock_vcs):
co = Checkout("SLUG", mock_repo)

assert co.changed == [join("en-US", "ok.ftl"), join("en-US", "inside.ftl")]


@patch("pontoon.sync.core.checkout.Checkout")
def test_get_checkouts(_):
with pytest.raises(Exception) as exc_info:
Expand Down
53 changes: 52 additions & 1 deletion pontoon/sync/tests/test_translations_to_repo.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from os import makedirs
from os import makedirs, symlink
from os.path import dirname, exists, join
from tempfile import TemporaryDirectory
from textwrap import dedent
Expand All @@ -19,6 +19,7 @@
from pontoon.sync.core.translations_to_repo import (
build_moz_l10n_resource,
sync_translations_to_repo,
update_changed_resources,
)
from pontoon.sync.tests.utils import build_file_tree
from pontoon.test.factories import (
Expand All @@ -36,6 +37,56 @@
now = timezone.now()


@pytest.mark.django_db
def test_target_outside_checkout_is_not_written():
"""An export only writes to paths that resolve inside the checkout."""
with TemporaryDirectory() as root:
# Database setup
settings.MEDIA_ROOT = root
locale = LocaleFactory.create(code="fr-Test")
repo = RepositoryFactory(url="http://example.com/repo")
project = ProjectFactory.create(
name="test-containment", locales=[locale], repositories=[repo]
)
res = ResourceFactory.create(
project=project, path="messages.properties", format="properties"
)
entity = EntityFactory.create(resource=res, string="key=Value")
TranslationFactory.create(
entity=entity, locale=locale, string="key=Valeur", approved=True
)

# Filesystem setup: the target file is a symlink out of the checkout
elsewhere = join(root, "elsewhere.properties")
with open(elsewhere, "w") as file:
file.write("key=Unchanged\n")
makedirs(repo.checkout_path)
build_file_tree(
repo.checkout_path,
{"en-US": {"messages.properties": "key=Value\n"}, "fr-Test": {}},
)
symlink(elsewhere, join(repo.checkout_path, "fr-Test", "messages.properties"))

# Paths setup
mock_checkout = Mock(
Checkout, path=repo.checkout_path, changed=[], removed=[], renamed=[]
)
paths = find_paths(project, Checkouts(mock_checkout, mock_checkout))

update_changed_resources(
project,
paths,
{locale.code: locale},
[],
ChangedEntityLocale.objects.filter(entity=entity),
set(),
now,
)

with open(elsewhere) as file:
assert file.read() == "key=Unchanged\n", "wrote outside the checkout"


@pytest.mark.django_db
def test_remove_resource():
with TemporaryDirectory() as root:
Expand Down