diff --git a/cloudpebble/ide/api/project.py b/cloudpebble/ide/api/project.py index feda84d..70be457 100644 --- a/cloudpebble/ide/api/project.py +++ b/cloudpebble/ide/api/project.py @@ -19,7 +19,8 @@ from ide.tasks.archive import create_archive, do_import_archive from ide.tasks.build import run_compile from ide.tasks.gist import import_gist -from ide.tasks.git import do_import_github +from ide.tasks.git import do_import_github, branch_exists +from ide.utils.github_urls import parse_github_source, split_ref_qualifier from ide.utils.alloy_templates import list_alloy_templates, build_template_archive from ide.utils.c_templates import list_c_templates, build_c_template_archive from utils.td_helper import send_td_event @@ -1029,14 +1030,55 @@ def import_zip(request): def import_github(request): name = request.POST['name'] repo = request.POST['repo'] - branch = request.POST['branch'] + branch = request.POST.get('branch', '') add_remote = (request.POST['add_remote'] == 'true') - match = re.match(r'^(?:https?://|git@|git://)?(?:www\.)?github\.com[/:]([\w.-]+)/([\w.-]+?)(?:\.git|/|$)', repo) - if match is None: + source = parse_github_source(repo) + if source is None: raise BadRequest(_("Invalid Github URL.")) - github_user = match.group(1) - github_project = match.group(2) + github_user = source.user + github_project = source.project + refpath, kind = source.refpath, source.kind + if refpath and kind == 'tree' and '/' not in refpath: + # A slash-free /tree/ is unambiguously a branch or tag — exactly + # equivalent to typing it into the branch box, so every existing flow + # stays available. Linking is the exception: a linked remote does + # pull/push through repo.get_branch(), so a TAG must not be stored as + # github_branch — probe first, and refuse only on a definite "not a + # branch" (an unanswerable probe keeps the branch-box behavior, which + # never validated either). + if add_remote and branch_exists(request.user, github_user, github_project, refpath) is False: + raise BadRequest(_("Linking a repository requires a branch; '%s' is not one.") % refpath) + branch, refpath, kind = refpath, None, None + elif refpath: + # The URL is authoritative for both the ref and the subdirectory (the + # celery task resolves the ambiguous split against the repository's + # real refs); a separately-typed branch would conflict, so it is + # ignored. + branch = '' + if add_remote: + if kind == 'tree': + # A slashed remainder may still be nothing but a branch name + # ('feature/foo', or the refs/heads-qualified spelling of + # one) — probe the repository before refusing to link. One + # branch lookup, deliberately not the paginate-every-ref + # list, which is unbounded inside a web request. When the + # probe can't answer, the refusal below stays, conservatively. + namespaces, candidate = split_ref_qualifier(refpath) + if 'heads' in namespaces and branch_exists( + request.user, github_user, github_project, candidate): + branch, refpath, kind = candidate, None, None + if refpath is not None: + # Deliberately NOT suggesting to link later from settings: + # the project model has no notion of a subdirectory, so a + # linked push would re-find a project root in the full + # repository tree and could overwrite a different project + # there. + if kind == 'commit': + raise BadRequest(_("Linking a repository is not supported for commit imports.")) + if kind == 'blob': + raise BadRequest(_("Linking a repository is not supported for file-link imports.")) + raise BadRequest(_("Linking a repository is not supported for subdirectory imports.")) try: project = Project.objects.create(owner=request.user, name=name) @@ -1045,10 +1087,11 @@ def import_github(request): if add_remote: project.github_repo = "%s/%s" % (github_user, github_project) - project.github_branch = branch + project.github_branch = branch or None project.save() - task = do_import_github.delay(project.id, github_user, github_project, branch, delete_project=True) + task = do_import_github.delay(project.id, github_user, github_project, branch, delete_project=True, + github_refpath=refpath, github_kind=kind) return {'task_id': task.task_id, 'project_id': project.id} diff --git a/cloudpebble/ide/static/ide/js/__tests__/github-import-prefill.test.js b/cloudpebble/ide/static/ide/js/__tests__/github-import-prefill.test.js new file mode 100644 index 0000000..76d8d62 --- /dev/null +++ b/cloudpebble/ide/static/ide/js/__tests__/github-import-prefill.test.js @@ -0,0 +1,179 @@ +import { describe, it, expect, vi } from 'vitest'; +import { readFileSync } from 'fs'; +import { resolve } from 'path'; + +// project_list.js is one $(function(){...}) block; the jQuery mock runs the +// callback immediately, so loading the file executes the deep-link prefill. +// Every jQuery method is a chainable no-op except val(), which records per +// selector — enough to assert what lands in the import dialog's fields. +function makeJqueryMock(attrs) { + var elements = {}; + var $ = function(selector) { + if (typeof selector === 'function') { + selector(); + return $; + } + if (!elements[selector]) { + var store = { value: undefined, text: '', clickHandler: null }; + var el = new Proxy(store, { + get: function(target, prop) { + if (prop === 'val') { + return function(v) { + if (v === undefined) return target.value; + target.value = v; + return el; + }; + } + if (prop === 'text') { + return function(v) { + if (v === undefined) return target.text; + target.text = v; + return el; + }; + } + if (prop === 'click') { + return function(fn) { + if (typeof fn === 'function') target.clickHandler = fn; + else if (target.clickHandler) target.clickHandler(); + return el; + }; + } + if (prop === 'find') return function(sel) { return $(sel); }; + if (prop === 'attr') { + return function(name) { + if ((attrs || {})[selector]) return attrs[selector][name]; + return el; + }; + } + if (prop === 'is') return function() { return false; }; + if (prop === 'length') return 0; + if (prop === '_isMock') return true; + return function() { return el; }; + } + }); + elements[selector] = el; + } + return elements[selector]; + }; + $.Deferred = vi.fn(); + $.elements = elements; + return $; +} + +function load(pathname, attrs) { + var code = readFileSync(resolve(__dirname, '..', 'project_list.js'), 'utf8'); + var $ = makeJqueryMock(attrs); + var chain = { then: function() { return chain; }, catch: function() { return chain; } }; + var ajax = { Post: vi.fn(function() { return chain; }), PollTask: vi.fn() }; + var fn = new Function( + '$', 'jQuery', 'gettext', 'jquery_csrf_setup', 'ga', 'Ajax', 'CloudPebble', 'location', + code + ); + fn($, $, function(s) { return s; }, vi.fn(), vi.fn(), ajax, {}, { pathname: pathname }); + return { $: $, ajax: ajax }; +} + +function loadWithPath(pathname) { + var h = load(pathname); + return { + name: h.$('#import-github-name').val(), + url: h.$('#import-github-url').val(), + branch: h.$('#import-github-branch').val() + }; +} + +// Drives the real import dialog: the active tab claims to be the GitHub +// pane, fields are set, and the Run button's captured handler is fired. +function submitGithubImport(fields) { + var h = load('/ide/', { '#import-prompt .tab-pane.active': { id: 'import-github' } }); + h.$('#import-github-name').val(fields.name); + h.$('#import-github-url').val(fields.url); + h.$('#import-github-branch').val(fields.branch); + h.$('#run-import').click(); + return h; +} + +describe('GitHub import deep-link prefill', () => { + it('hands a /tree// URL (the appstore Remix shape) to the server whole', () => { + var fields = loadWithPath('/ide/import/github/emindeniz99/pebble-signals/tree/main/faces/slothvec'); + expect(fields.url).toBe('github.com/emindeniz99/pebble-signals/tree/main/faces/slothvec'); + expect(fields.name).toBe('slothvec'); + // The URL is authoritative for the ref — the branch box stays empty. + expect(fields.branch).toBeUndefined(); + }); + + it('suggests the repo name when a /tree/ URL has no subdirectory', () => { + var fields = loadWithPath('/ide/import/github/user/repo/tree/main'); + expect(fields.url).toBe('github.com/user/repo/tree/main'); + expect(fields.name).toBe('repo'); + expect(fields.branch).toBeUndefined(); + }); + + it('suggests the repo name for /blob/ and /commit/ URLs (tail is a file or SHA, not a name)', () => { + var blob = loadWithPath('/ide/import/github/user/repo/blob/main/src/app.js'); + expect(blob.url).toBe('github.com/user/repo/blob/main/src/app.js'); + expect(blob.name).toBe('repo'); + var commit = loadWithPath('/ide/import/github/user/repo/commit/abc123'); + expect(commit.url).toBe('github.com/user/repo/commit/abc123'); + expect(commit.name).toBe('repo'); + }); + + it('decodes the suggested name but keeps the URL encoded', () => { + var fields = loadWithPath('/ide/import/github/user/repo/tree/main/my%20dir'); + expect(fields.url).toBe('github.com/user/repo/tree/main/my%20dir'); + expect(fields.name).toBe('my dir'); + }); + + it('keeps the legacy /// deep-link contract', () => { + var fields = loadWithPath('/ide/import/github/user/repo/dev'); + expect(fields.url).toBe('github.com/user/repo'); + // parts[3] is the USERNAME — a pre-existing upstream quirk, kept + // bug-compatible on purpose (this PR only preserves the contract). + expect(fields.name).toBe('user'); + expect(fields.branch).toBe('dev'); + }); + + it('keeps legacy slashed-branch deep links', () => { + var fields = loadWithPath('/ide/import/github/user/repo/feat/x'); + expect(fields.url).toBe('github.com/user/repo'); + expect(fields.branch).toBe('feat/x'); + }); + + it('leaves the branch empty for a plain repo link (server imports the default branch)', () => { + var fields = loadWithPath('/ide/import/github/user/repo'); + expect(fields.url).toBe('github.com/user/repo'); + expect(fields.branch).toBeUndefined(); + }); + + it('does nothing outside the import deep link', () => { + var fields = loadWithPath('/ide/'); + expect(fields.url).toBeUndefined(); + expect(fields.name).toBeUndefined(); + }); +}); + + +describe('GitHub import submit handler', () => { + it('submits an empty branch untouched (no master fallback)', () => { + var h = submitGithubImport({ name: 'proj', url: 'github.com/user/repo', branch: '' }); + expect(h.ajax.Post).toHaveBeenCalledWith('/ide/import/github', { + name: 'proj', repo: 'github.com/user/repo', branch: '', add_remote: false + }); + }); + + it('lets /tree/ URLs through the prefix check to the server', () => { + var h = submitGithubImport({ + name: 'sloth', url: 'github.com/user/repo/tree/main/faces/slothvec', branch: '' + }); + expect(h.ajax.Post).toHaveBeenCalledWith('/ide/import/github', { + name: 'sloth', repo: 'github.com/user/repo/tree/main/faces/slothvec', + branch: '', add_remote: false + }); + }); + + it('rejects a non-GitHub URL before posting', () => { + var h = submitGithubImport({ name: 'p', url: 'gitlab.com/user/repo', branch: '' }); + expect(h.ajax.Post).not.toHaveBeenCalled(); + expect(h.$('.errors').text()).toContain('GitHub'); + }); +}); diff --git a/cloudpebble/ide/static/ide/js/project_list.js b/cloudpebble/ide/static/ide/js/project_list.js index 564b9ab..d0f74dd 100644 --- a/cloudpebble/ide/static/ide/js/project_list.js +++ b/cloudpebble/ide/static/ide/js/project_list.js @@ -178,14 +178,14 @@ $(function() { active_set.find('.errors').removeClass('hide').text(gettext("You must specify a project name.")); return; } - // This is identical to the regex used on the server. + // A prefix check only — the server (ide.utils.github_urls) is the + // authority and also understands /tree// and /blob/ URLs. if(!/^(?:https?:\/\/|git@|git:\/\/)?(?:www\.)?github\.com[\/:]([\w.-]+)\/([\w.-]+?)(?:\.git|\/|$)/.test(url)) { active_set.find('.errors').removeClass('hide').text(gettext("You must specify a complete GitHub project URL")); return; } - if(branch.length == 0) { - branch = 'master'; - } + // An empty branch imports the repository's default branch (the old + // hardcoded 'master' fallback broke every main-default repository). disable_import_controls(); active_set.find('.progress').removeClass('hide'); do_import(Ajax.Post('/ide/import/github', { @@ -227,10 +227,29 @@ $(function() { if (path.indexOf('/ide/import/github/') === 0) { var parts = path.substr(1).split('/'); $('#import-prompt').modal(); - $('#import-github-name').val(parts[3]); - $('#import-github-url').val('github.com/' + parts[3] + '/' + parts[4]); - if (parts.length > 5) { - $('#import-github-branch').val(parts.slice(5).join('/')); + var tail = parts.slice(5).filter(function(p) { return p.length > 0; }); + if (tail.length > 1 && (tail[0] == 'tree' || tail[0] == 'blob' || tail[0] == 'commit')) { + // A GitHub web URL shape (e.g. the appstore's "Remix" button: + // .../import/github///tree//). Hand + // the whole thing to the server, which resolves branch vs + // subdirectory against the repository's real refs. + // Suggest the subdirectory name for /tree/ URLs (it may still be + // part of a slashed branch name — the field is editable and the + // server resolves the truth); blob/commit tails are a filename or + // a SHA, so the repository name is the sane suggestion there. + var suggested = (tail[0] == 'tree' && tail.length > 2) ? tail[tail.length - 1] : parts[4]; + // location.pathname keeps percent-encoding; the URL field must + // stay encoded (the server decodes it) but a suggested NAME + // should read as text. Malformed encodings just stay as-is. + try { suggested = decodeURIComponent(suggested); } catch (e) {} + $('#import-github-name').val(suggested); + $('#import-github-url').val('github.com/' + parts.slice(3).join('/')); + } else { + $('#import-github-name').val(parts[3]); + $('#import-github-url').val('github.com/' + parts[3] + '/' + parts[4]); + if (tail.length) { + $('#import-github-branch').val(tail.join('/')); + } } $('a[href=#import-github]').tab('show'); } diff --git a/cloudpebble/ide/tasks/archive.py b/cloudpebble/ide/tasks/archive.py index 3f1fe0b..ae41f01 100644 --- a/cloudpebble/ide/tasks/archive.py +++ b/cloudpebble/ide/tasks/archive.py @@ -25,6 +25,14 @@ logger = logging.getLogger(__name__) +# Hard limit on how many files an imported project may contain. +PROJECT_ENTRY_LIMIT = 400 + +# When a root_hint narrows an import to one project, PROJECT_ENTRY_LIMIT +# applies to that subtree — but the whole archive still needs a ceiling to +# bound the root-finding scan over the repository it lives in. +HINTED_ARCHIVE_SCAN_LIMIT = 50000 + def _public_export_url(path): return "/ide/export/%s" % path.lstrip('/') @@ -145,7 +153,7 @@ def ends_with_any(s, options): @shared_task(acks_late=True) -def do_import_archive(project_id, archive, delete_project=False, wipe_existing=False): +def do_import_archive(project_id, archive, delete_project=False, wipe_existing=False, root_hint=None): project = Project.objects.get(pk=project_id) try: with tempfile.NamedTemporaryFile(suffix='.zip') as archive_file: @@ -168,11 +176,18 @@ def do_import_archive(project_id, archive, delete_project=False, wipe_existing=F WORKER_SRC_DIR = 'worker_src/' INCLUDE_SRC_DIR = 'include/' - if len(contents) > 400: + if len(contents) > (PROJECT_ENTRY_LIMIT if root_hint is None else HINTED_ARCHIVE_SCAN_LIMIT): raise InvalidProjectArchiveException("Too many files in zip file.") archive_items = [ArchiveProjectItem(z, x) for x in contents] - base_dir, manifest_item = find_project_root_and_manifest(archive_items) + base_dir, manifest_item = find_project_root_and_manifest(archive_items, root_hint=root_hint) + if root_hint is not None: + # A subdirectory import only takes files under the hinted + # project, so the size limit applies to that subtree — not + # to the whole repository the project happens to live in. + contents = [entry for entry in contents if entry.filename.startswith(base_dir)] + if len(contents) > PROJECT_ENTRY_LIMIT: + raise InvalidProjectArchiveException("Too many files in the project directory.") dir_end = len(base_dir) def make_valid_filename(zip_entry): diff --git a/cloudpebble/ide/tasks/git.py b/cloudpebble/ide/tasks/git.py index dfd0d4a..236cb34 100644 --- a/cloudpebble/ide/tasks/git.py +++ b/cloudpebble/ide/tasks/git.py @@ -1,9 +1,11 @@ import base64 import io +import posixpath from concurrent.futures import ThreadPoolExecutor from io import BytesIO from urllib.request import urlopen, Request from urllib.error import URLError, HTTPError +from urllib.parse import quote import json import os import logging @@ -22,6 +24,7 @@ from ide.tasks import do_import_archive, run_compile from ide.tasks.archive import get_filename_variant from ide.utils.git import git_sha, git_blob +from ide.utils.github_urls import split_ref_qualifier, split_ref_and_path, normalize_subpath from ide.utils.project import find_project_root_and_manifest, BaseProjectItem, InvalidProjectArchiveException, MANIFEST_KINDS from ide.utils.sdk import generate_manifest_dict, generate_manifest, generate_wscript_file, load_manifest_dict, manifest_name_for_project from utils.td_helper import send_td_event @@ -31,6 +34,10 @@ logger = logging.getLogger(__name__) +# Longest branch name (in slash-separated segments) the codeload probe +# fallback will consider when the ref list is unavailable. +MAX_PROBE_REF_SEGMENTS = 10 + def exception_reason(error): reason = str(error) @@ -40,7 +47,8 @@ def exception_reason(error): @shared_task(acks_late=True) -def do_import_github(project_id, github_user, github_project, github_branch, delete_project=False): +def do_import_github(project_id, github_user, github_project, github_branch, delete_project=False, + github_refpath=None, github_kind=None): project = None user = None try: @@ -50,7 +58,22 @@ def do_import_github(project_id, github_user, github_project, github_branch, del except: pass - url = "https://github.com/%s/%s/archive/%s.zip" % (github_user, github_project, github_branch) + github_path = '' + if github_refpath: + # A /tree/ or /blob/ URL: the branch and the subdirectory are one + # ambiguous string until matched against the repository's refs. + github_branch, github_path = resolve_ref_and_path( + user, github_user, github_project, github_refpath, github_kind) + if not github_branch: + # codeload resolves HEAD to the repository's default branch, so an + # empty branch imports what visitors see — instead of the old + # hardcoded 'master', which broke every main-default repository. + github_branch = 'HEAD' + + # quote() the ref: slashes are real URL separators here, but a branch + # name may legally contain '#', '?' or spaces, which would otherwise + # truncate the request path. + url = "https://github.com/%s/%s/archive/%s.zip" % (github_user, github_project, quote(github_branch, safe='/')) auth_url = get_authenticated_archive_url(user, github_user, github_project, github_branch) archive = None @@ -68,7 +91,18 @@ def do_import_github(project_id, github_user, github_project, github_branch, del % (github_user, github_project, github_branch) ) - return do_import_archive(project_id, archive.read()) + # A /blob/ URL names a FILE, so its resolved directory — even the + # empty one, for a root-level file — is an explicit choice, and the + # empty path becomes the explicit repository-root hint. For /tree/ + # and /commit/ URLs an empty path just means "the whole tree": those + # keep the same first-manifest heuristic a branch-box import gets, + # so a repository whose only project is nested imports identically + # through either door. + if github_kind == 'blob': + root_hint = github_path + else: + root_hint = github_path or None + return do_import_archive(project_id, archive.read(), root_hint=root_hint) except Exception as e: if delete_project and project is not None: try: @@ -80,12 +114,107 @@ def do_import_github(project_id, github_user, github_project, github_branch, del 'reason': exception_reason(e), 'github_user': github_user, 'github_project': github_project, - 'github_branch': github_branch + 'github_branch': github_branch, + 'github_refpath': github_refpath } }, user=user) raise +def resolve_ref_and_path(user, github_user, github_project, refpath, kind): + """ Resolve a /tree/ or /blob/ remainder ("[/]") into a real + (ref, subdirectory) pair. Branch names may contain slashes, so the split + is decided against the repository's actual refs — via the GitHub API when + possible, else by probing which archive actually exists (longest candidate + first), the way gitpick and gitingest resolve GitHub web URLs. """ + if kind == 'commit': + return refpath, '' + + # A self-qualified refs/heads/... or refs/tags/... spelling pins the + # namespace; strip it HERE so the probe fallback below builds candidates + # without the qualifier (probing 'refs/heads/refs/heads/x' can never hit). + namespaces, refpath = split_ref_qualifier(refpath) + + ref, path = None, None + ref_names = get_ref_names(user, github_user, github_project, namespaces) + if ref_names is not None: + ref, path = split_ref_and_path(refpath, ref_names) + elif '/' in refpath: + # No API access (rate limit, private repo without a token): probe the + # public archive endpoint. Longest first, so 'feature/foo/src' tries + # branch 'feature/foo' before branch 'feature'. The refs-qualified + # codeload form MUST be used here: bare archive/.zip answers 200 + # for junk like 'main/anything', while zip/refs/heads/ is strict + # (measured: refs/heads/main 200, refs/heads/main/faces 404). + parts = refpath.split('/') + start = len(parts) if kind == 'tree' else len(parts) - 1 + # Bound the loop: no real branch name has anywhere near this many + # slash-separated segments, and without a cap an attacker-supplied + # deep URL would turn the probe into hundreds of sequential HEAD + # requests on the worker. Deep PATHS still resolve fine as long as + # the branch itself fits the cap. + for i in range(min(start, MAX_PROBE_REF_SEGMENTS), 0, -1): + candidate = '/'.join(parts[:i]) + if any(file_exists("https://codeload.github.com/%s/%s/zip/refs/%s/%s" + % (github_user, github_project, refkind, quote(candidate, safe='/'))) + for refkind in namespaces): + ref, path = candidate, '/'.join(parts[i:]) + break + if ref is None: + ref, path = split_ref_and_path(refpath, None) + + if kind == 'blob': + # A blob URL names a file; the project is the directory it lives in. + path = posixpath.dirname(path) + try: + path = normalize_subpath(path) + except ValueError: + raise Exception("Invalid project path in GitHub URL: '%s'" % refpath) + return ref, path + + +def branch_exists(user, github_user, github_project, branch): + """ True/False whether the branch exists, or None when the API is + unavailable. A constant two API calls — unlike get_ref_names, which + paginates every branch and tag and so must never run inside a web + request. """ + try: + try: + g = get_github(user) if user is not None else Github() + except Exception: + # No linked GitHub account — public repositories still answer + # anonymously. + g = Github() + repo = g.get_repo("%s/%s" % (github_user, github_project)) + try: + repo.get_branch(branch) + return True + except GithubException as e: + if e.status == 404: + return False + raise + except Exception: + return None + + +def get_ref_names(user, github_user, github_project, namespaces=('heads', 'tags')): + """ Ref names in the requested namespaces, or None when the API is + unavailable. A refs/heads|tags-qualified URL pins the namespace, and the + pin must reach the name list: with both merged, a tag could + longest-prefix-match a remainder the URL explicitly claimed as a branch. """ + try: + g = get_github(user) if user is not None else Github() + repo = g.get_repo("%s/%s" % (github_user, github_project)) + names = [] + if 'heads' in namespaces: + names += [b.name for b in repo.get_branches()] + if 'tags' in namespaces: + names += [t.name for t in repo.get_tags()] + return names + except Exception: + return None + + def file_exists(url): request = Request(url) request.get_method = lambda: 'HEAD' diff --git a/cloudpebble/ide/templates/ide/index.html b/cloudpebble/ide/templates/ide/index.html index 6de99ac..f69acf0 100644 --- a/cloudpebble/ide/templates/ide/index.html +++ b/cloudpebble/ide/templates/ide/index.html @@ -173,7 +173,7 @@

{% trans "Import Existing Project" %}

- +
{% if user.github_repo_sync %} @@ -202,5 +202,5 @@

{% trans "Import Existing Project" %}

{{ alloy_templates|json_script:"alloy-template-data" }} - + {% endblock %} diff --git a/cloudpebble/ide/tests/test_find_project_root.py b/cloudpebble/ide/tests/test_find_project_root.py index fd2fc67..ed790b6 100644 --- a/cloudpebble/ide/tests/test_find_project_root.py +++ b/cloudpebble/ide/tests/test_find_project_root.py @@ -149,3 +149,81 @@ def test_skip_build_and_node_modules_together(self): "project/src/c/main.c", "project/resources/fonts/font.ttf", ], "project/", "project/package.json") + + +class TestFindProjectRootWithHint(TestCase): + """ root_hint pins the search to one subdirectory — the /tree// + import case, where the repository may contain several projects. """ + + REPO = [ + # A repository that is ALSO a valid project at its root (a library + # shipping its own demo), plus two nested projects — the shape that + # made unhinted imports pick the wrong one. + "repo-main/package.json", + "repo-main/src/", + "repo-main/faces/slothvec/package.json", + "repo-main/faces/slothvec/src/", + "repo-main/examples/other/package.json", + ] + + def find(self, contents, root_hint): + return find_project_root_and_manifest( + [FakeProjectItem(item) for item in contents], root_hint=root_hint) + + def test_hint_picks_nested_project_over_root(self): + base_dir, manifest = self.find(self.REPO, "faces/slothvec") + self.assertEqual(base_dir, "repo-main/faces/slothvec/") + self.assertEqual(manifest.name, "repo-main/faces/slothvec/package.json") + + def test_hint_matches_archive_without_wrapper_dir(self): + base_dir, manifest = self.find( + ["faces/slothvec/package.json", "faces/slothvec/src/"], "faces/slothvec") + self.assertEqual(base_dir, "faces/slothvec/") + + def test_hint_with_no_project_there_throws(self): + with self.assertRaises(InvalidProjectArchiveException): + self.find(self.REPO, "faces/missing") + + def test_hint_does_not_match_deeper_manifests(self): + with self.assertRaises(InvalidProjectArchiveException): + self.find(self.REPO, "faces") + + def test_hint_suffix_alone_is_not_enough(self): + # 'slothvec' names the right LEAF, but the project lives at + # faces/slothvec — a lazy suffix match would import it anyway and + # mask a wrong ref/path split upstream. It must fail loudly instead. + with self.assertRaises(InvalidProjectArchiveException): + self.find(self.REPO, "slothvec") + + def test_empty_hint_is_the_explicit_repository_root(self): + # A /blob// URL selects the repository root + # explicitly; the heuristic must not wander into nested projects. + base_dir, manifest = self.find(self.REPO, "") + self.assertEqual(base_dir, "repo-main/") + self.assertEqual(manifest.name, "repo-main/package.json") + + def test_empty_hint_without_root_project_throws(self): + with self.assertRaises(InvalidProjectArchiveException): + self.find(["repo-main/faces/slothvec/package.json", + "repo-main/faces/slothvec/src/"], "") + + def test_hint_never_reads_manifests_outside_it(self): + # The hint must narrow BEFORE manifest contents are read, or a huge + # repository's out-of-tree manifests get opened and parsed for + # nothing. + read_paths = [] + + class RecordingItem(FakeProjectItem): + def read(inner): + read_paths.append(inner.name) + return super(RecordingItem, inner).read() + + base_dir, manifest = find_project_root_and_manifest( + [RecordingItem(item) for item in self.REPO], root_hint="faces/slothvec") + self.assertEqual(base_dir, "repo-main/faces/slothvec/") + self.assertEqual(read_paths, ["repo-main/faces/slothvec/package.json"]) + + def test_no_hint_keeps_existing_behavior(self): + base_dir, manifest = find_project_root_and_manifest( + [FakeProjectItem(item) for item in self.REPO]) + self.assertEqual(base_dir, "repo-main/") diff --git a/cloudpebble/ide/tests/test_github_import.py b/cloudpebble/ide/tests/test_github_import.py new file mode 100644 index 0000000..c255f58 --- /dev/null +++ b/cloudpebble/ide/tests/test_github_import.py @@ -0,0 +1,337 @@ +""" Tests for the GitHub-import ref/path resolution in ide.tasks.git — +resolve_ref_and_path() with the GitHub API mocked out, and the strict +codeload probe fallback with file_exists mocked — plus the /ide/import/github +API flows around linking (add_remote). """ + +import json +from unittest import mock + +from django.test import TestCase + +from ide.models.project import Project +from ide.tasks.git import do_import_github, resolve_ref_and_path +from ide.utils.cloudpebble_test import CloudpebbleTestCase + + +@mock.patch('ide.tasks.git.get_ref_names') +class TestResolveRefAndPath(TestCase): + REFS = ['main', 'develop', 'feature/foo', 'v1.0.0'] + + def test_api_split(self, get_ref_names): + get_ref_names.return_value = self.REFS + self.assertEqual(('main', 'faces/slothvec'), + resolve_ref_and_path(None, 'u', 'r', 'main/faces/slothvec', 'tree')) + + def test_api_split_slashed_branch(self, get_ref_names): + get_ref_names.return_value = self.REFS + self.assertEqual(('feature/foo', 'src'), + resolve_ref_and_path(None, 'u', 'r', 'feature/foo/src', 'tree')) + + def test_blob_resolves_to_the_files_directory(self, get_ref_names): + get_ref_names.return_value = self.REFS + self.assertEqual(('main', 'faces'), + resolve_ref_and_path(None, 'u', 'r', 'main/faces/app.js', 'blob')) + + def test_commit_passthrough(self, get_ref_names): + self.assertEqual(('abc1234', ''), + resolve_ref_and_path(None, 'u', 'r', 'abc1234', 'commit')) + get_ref_names.assert_not_called() + + def test_invalid_path_fails_loud(self, get_ref_names): + get_ref_names.return_value = self.REFS + with self.assertRaisesRegex(Exception, 'Invalid project path'): + resolve_ref_and_path(None, 'u', 'r', 'main/../etc', 'tree') + + @mock.patch('ide.tasks.git.file_exists') + def test_probe_fallback_longest_first(self, file_exists, get_ref_names): + get_ref_names.return_value = None + probed = [] + + def fake_exists(url): + probed.append(url) + return url.endswith('/zip/refs/heads/feature/foo') + file_exists.side_effect = fake_exists + + self.assertEqual(('feature/foo', 'src'), + resolve_ref_and_path(None, 'u', 'r', 'feature/foo/src', 'tree')) + # Longest candidate first, strict refs-qualified codeload URLs only. + self.assertIn('https://codeload.github.com/u/r/zip/refs/heads/feature/foo/src', probed) + self.assertTrue(all('/zip/refs/' in url for url in probed)) + + @mock.patch('ide.tasks.git.file_exists') + def test_probe_strips_refs_qualifier_and_pins_namespace(self, file_exists, get_ref_names): + get_ref_names.return_value = None + probed = [] + + def fake_exists(url): + probed.append(url) + return url.endswith('/zip/refs/heads/feature/foo') + file_exists.side_effect = fake_exists + + # refs/heads/feature/foo/src: candidates must NOT start with refs/heads/ + # a second time, and refs/tags/ must never be probed. + self.assertEqual(('feature/foo', 'src'), + resolve_ref_and_path(None, 'u', 'r', 'refs/heads/feature/foo/src', 'tree')) + self.assertTrue(all('/zip/refs/heads/' in url for url in probed)) + self.assertTrue(all('refs/heads/refs' not in url for url in probed)) + + def test_qualified_ref_pins_the_namespace_on_the_api_path(self, get_ref_names): + # Branch 'release' and tag 'release/docs' both exist (legal + # cross-namespace names). A refs/heads-qualified URL must only be + # matched against branches, or the tag longest-prefix-wins wrongly. + def by_namespace(user, gh_user, gh_project, namespaces=('heads', 'tags')): + names = [] + if 'heads' in namespaces: + names += ['main', 'release'] + if 'tags' in namespaces: + names += ['release/docs', 'v1.0.0'] + return names + get_ref_names.side_effect = by_namespace + + self.assertEqual(('release', 'docs/app'), + resolve_ref_and_path(None, 'u', 'r', 'refs/heads/release/docs/app', 'tree')) + self.assertEqual(('release/docs', 'app'), + resolve_ref_and_path(None, 'u', 'r', 'refs/tags/release/docs/app', 'tree')) + # Unqualified stays cross-namespace, longest first. + self.assertEqual(('release/docs', 'app'), + resolve_ref_and_path(None, 'u', 'r', 'release/docs/app', 'tree')) + + @mock.patch('ide.tasks.git.file_exists') + def test_probe_quotes_unsafe_ref_characters(self, file_exists, get_ref_names): + # A branch may legally contain '#' or '?'; raw in a probe URL they + # would truncate the request path into a fragment/query. + get_ref_names.return_value = None + probed = [] + + def fake_exists(url): + probed.append(url) + return url.endswith('/zip/refs/heads/bug%237') + file_exists.side_effect = fake_exists + + self.assertEqual(('bug#7', 'src'), + resolve_ref_and_path(None, 'u', 'r', 'bug#7/src', 'tree')) + self.assertTrue(all('#' not in url for url in probed)) + + @mock.patch('ide.tasks.git.file_exists') + def test_probe_is_bounded_for_deep_refpaths(self, file_exists, get_ref_names): + # An attacker-supplied 60-segment URL must not turn the fallback + # into 60+ sequential probe requests on the worker. + get_ref_names.return_value = None + file_exists.return_value = False + deep = '/'.join('seg%d' % i for i in range(60)) + self.assertEqual(('seg0', '/'.join('seg%d' % i for i in range(1, 60))), + resolve_ref_and_path(None, 'u', 'r', deep, 'tree')) + self.assertLessEqual(file_exists.call_count, 20) + + @mock.patch('ide.tasks.git.file_exists') + def test_probe_miss_falls_back_to_first_segment(self, file_exists, get_ref_names): + get_ref_names.return_value = None + file_exists.return_value = False + self.assertEqual(('main', 'faces/slothvec'), + resolve_ref_and_path(None, 'u', 'r', 'main/faces/slothvec', 'tree')) + + +@mock.patch('ide.api.project.do_import_github') +class TestImportGithubApi(CloudpebbleTestCase): + """ The linking (add_remote) rules of POST /ide/import/github. """ + + def setUp(self): + self.login() + + def import_repo(self, do_import_github, repo, add_remote='false', branch=''): + do_import_github.delay.return_value.task_id = 'task-id' + return self.client.post('/ide/import/github', { + 'name': 'imported', 'repo': repo, 'branch': branch, 'add_remote': add_remote}) + + @mock.patch('ide.api.project.branch_exists') + def test_branch_only_tree_url_keeps_linking(self, branch_exists, do_import_github): + branch_exists.return_value = True + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/main', add_remote='true').content) + self.assertTrue(result['success'], msg=result.get('error')) + project = Project.objects.get(pk=result['project_id']) + self.assertEqual(project.github_repo, 'u/r') + self.assertEqual(project.github_branch, 'main') + args, kwargs = do_import_github.delay.call_args + self.assertEqual(args[3], 'main') + self.assertIsNone(kwargs['github_refpath']) + + @mock.patch('ide.api.project.branch_exists') + def test_subdirectory_with_linking_is_rejected(self, branch_exists, do_import_github): + branch_exists.return_value = False + response = self.import_repo( + do_import_github, 'github.com/u/r/tree/main/faces/x', add_remote='true') + self.assertEqual(response.status_code, 400) + self.assertIn('subdirectory imports', json.loads(response.content)['error']) + do_import_github.delay.assert_not_called() + + def test_commit_with_linking_is_rejected_with_its_own_reason(self, do_import_github): + response = self.import_repo( + do_import_github, 'github.com/u/r/commit/abc123', add_remote='true') + self.assertEqual(response.status_code, 400) + self.assertIn('commit imports', json.loads(response.content)['error']) + do_import_github.delay.assert_not_called() + + @mock.patch('ide.api.project.branch_exists') + def test_slashed_branch_only_tree_url_keeps_linking(self, branch_exists, do_import_github): + # /tree/feature/foo where feature/foo is purely a branch: one branch + # probe must keep the linking flow available. + branch_exists.return_value = True + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/feature/foo', add_remote='true').content) + self.assertTrue(result['success'], msg=result.get('error')) + project = Project.objects.get(pk=result['project_id']) + self.assertEqual(project.github_branch, 'feature/foo') + args, kwargs = do_import_github.delay.call_args + self.assertEqual(args[3], 'feature/foo') + self.assertIsNone(kwargs['github_refpath']) + + @mock.patch('ide.api.project.branch_exists') + def test_slash_free_tag_with_linking_is_rejected(self, branch_exists, do_import_github): + # /tree/v1.0.0 + linking: the import would work (archives accept + # tags), but a linked remote pulls/pushes via get_branch() — a + # definite "not a branch" must refuse linking up front. + branch_exists.return_value = False + response = self.import_repo( + do_import_github, 'github.com/u/r/tree/v1.0.0', add_remote='true') + self.assertEqual(response.status_code, 400) + self.assertIn('requires a branch', json.loads(response.content)['error']) + do_import_github.delay.assert_not_called() + + @mock.patch('ide.api.project.branch_exists') + def test_slash_free_linking_stays_available_when_probe_cannot_answer(self, branch_exists, do_import_github): + # Fail-open on an unanswerable probe: the branch box never validated + # either, and /tree/main must keep linking under a rate limit. + branch_exists.return_value = None + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/main', add_remote='true').content) + self.assertTrue(result['success'], msg=result.get('error')) + project = Project.objects.get(pk=result['project_id']) + self.assertEqual(project.github_branch, 'main') + + @mock.patch('ide.api.project.branch_exists') + def test_qualified_branch_tree_url_keeps_linking(self, branch_exists, do_import_github): + # The refs/heads-qualified spelling of the same URL must link too — + # and the probe must see the UNQUALIFIED name. + branch_exists.return_value = True + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/refs/heads/feature/foo', add_remote='true').content) + self.assertTrue(result['success'], msg=result.get('error')) + self.assertEqual(branch_exists.call_args[0][3], 'feature/foo') + project = Project.objects.get(pk=result['project_id']) + self.assertEqual(project.github_branch, 'feature/foo') + + @mock.patch('ide.api.project.branch_exists') + def test_tags_qualified_tree_url_rejects_linking(self, branch_exists, do_import_github): + # A tag is not a branch; linking to one makes no sense and the + # probe must not even run. + response = self.import_repo( + do_import_github, 'github.com/u/r/tree/refs/tags/v1.0.0', add_remote='true') + self.assertEqual(response.status_code, 400) + branch_exists.assert_not_called() + + @mock.patch('ide.api.project.branch_exists') + def test_slashed_subdirectory_still_rejects_linking(self, branch_exists, do_import_github): + branch_exists.return_value = False + response = self.import_repo( + do_import_github, 'github.com/u/r/tree/feature/foo/src', add_remote='true') + self.assertEqual(response.status_code, 400) + self.assertIn('subdirectory imports', json.loads(response.content)['error']) + + @mock.patch('ide.api.project.branch_exists') + def test_slashed_tree_rejects_linking_when_probe_unavailable(self, branch_exists, do_import_github): + # Probe can't answer (rate limit, private repo): stay conservative. + branch_exists.return_value = None + response = self.import_repo( + do_import_github, 'github.com/u/r/tree/feature/foo', add_remote='true') + self.assertEqual(response.status_code, 400) + do_import_github.delay.assert_not_called() + + def test_blob_with_linking_gets_its_own_reason(self, do_import_github): + response = self.import_repo( + do_import_github, 'github.com/u/r/blob/main/package.json', add_remote='true') + self.assertEqual(response.status_code, 400) + self.assertIn('file-link imports', json.loads(response.content)['error']) + + def test_url_refpath_wins_over_a_typed_branch(self, do_import_github): + # The URL is authoritative: a conflicting typed branch is dropped. + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/main/faces/x', branch='dev').content) + self.assertTrue(result['success'], msg=result.get('error')) + args, kwargs = do_import_github.delay.call_args + self.assertEqual(args[3], '') + self.assertEqual(kwargs['github_refpath'], 'main/faces/x') + + def test_branch_only_tree_url_wins_over_a_typed_branch(self, do_import_github): + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/main', branch='dev').content) + self.assertTrue(result['success'], msg=result.get('error')) + args, kwargs = do_import_github.delay.call_args + self.assertEqual(args[3], 'main') + + def test_subdirectory_without_linking_passes_the_refpath(self, do_import_github): + result = json.loads(self.import_repo( + do_import_github, 'github.com/u/r/tree/main/faces/x').content) + self.assertTrue(result['success'], msg=result.get('error')) + args, kwargs = do_import_github.delay.call_args + self.assertEqual(args[3], '') + self.assertEqual(kwargs['github_refpath'], 'main/faces/x') + self.assertEqual(kwargs['github_kind'], 'tree') + + +@mock.patch('ide.tasks.git.do_import_archive') +@mock.patch('ide.tasks.git.urlopen') +@mock.patch('ide.tasks.git.file_exists') +@mock.patch('ide.tasks.git.get_ref_names') +class TestDoImportGithubGlue(CloudpebbleTestCase): + """ The glue in do_import_github itself: which archive URL is fetched + and which root_hint reaches do_import_archive, per URL kind. """ + + def setUp(self): + self.login() + + def run_import(self, get_ref_names, file_exists, urlopen, do_import_archive, + branch='', refpath=None, kind=None, refs=('main',)): + get_ref_names.return_value = list(refs) + file_exists.return_value = True + urlopen.return_value.read.return_value = b'zipbytes' + do_import_github(self.project_id, 'u', 'r', branch, + github_refpath=refpath, github_kind=kind) + url = file_exists.call_args[0][0] + root_hint = do_import_archive.call_args[1]['root_hint'] + return url, root_hint + + def test_empty_branch_imports_the_default_branch(self, *mocks): + # The headline fix: codeload HEAD, not a hardcoded 'master'. + url, root_hint = self.run_import(*mocks) + self.assertTrue(url.endswith('/u/r/archive/HEAD.zip'), url) + self.assertIsNone(root_hint) + + def test_typed_branch_is_fetched_verbatim(self, *mocks): + url, root_hint = self.run_import(*mocks, branch='dev') + self.assertTrue(url.endswith('/u/r/archive/dev.zip'), url) + self.assertIsNone(root_hint) + + def test_tree_with_subdirectory_hints_the_subdirectory(self, *mocks): + url, root_hint = self.run_import(*mocks, refpath='main/faces/x', kind='tree') + self.assertTrue(url.endswith('/u/r/archive/main.zip'), url) + self.assertEqual(root_hint, 'faces/x') + + def test_blob_at_root_hints_the_explicit_root(self, *mocks): + url, root_hint = self.run_import(*mocks, refpath='main/package.json', kind='blob') + self.assertEqual(root_hint, '') + + def test_commit_gets_no_hint(self, *mocks): + # A commit URL selects a revision, not a directory — nested-project + # repos must import exactly like a branch-box import. + url, root_hint = self.run_import(*mocks, refpath='abc123', kind='commit') + self.assertTrue(url.endswith('/u/r/archive/abc123.zip'), url) + self.assertIsNone(root_hint) + + def test_pure_slashed_branch_tree_gets_no_hint(self, *mocks): + # /tree/feature/foo (whole remainder is a branch) must behave like + # typing feature/foo into the branch box. + url, root_hint = self.run_import(*mocks, refpath='feature/foo', kind='tree', + refs=('feature/foo',)) + self.assertTrue(url.endswith('/u/r/archive/feature/foo.zip'), url) + self.assertIsNone(root_hint) diff --git a/cloudpebble/ide/tests/test_github_urls.py b/cloudpebble/ide/tests/test_github_urls.py new file mode 100644 index 0000000..7595ebb --- /dev/null +++ b/cloudpebble/ide/tests/test_github_urls.py @@ -0,0 +1,179 @@ +""" Tests for ide.utils.github_urls — the GitHub source parser behind the +import dialog and the /ide/import/github/... deep links (including the +appstore's "Remix on CloudPebble" button URLs). Dependency-free on purpose: + python -m unittest ide.tests.test_github_urls +""" + +from unittest import TestCase + +from ide.utils.github_urls import parse_github_source, split_ref_qualifier, split_ref_and_path, normalize_subpath + + +class TestParseGithubSource(TestCase): + def assert_parsed(self, source, user, project, kind=None, refpath=None): + parsed = parse_github_source(source) + self.assertIsNotNone(parsed, "expected %r to parse" % source) + self.assertEqual( + (user, project, kind, refpath), + (parsed.user, parsed.project, parsed.kind, parsed.refpath), + ) + + def test_bare_shorthand(self): + self.assert_parsed('Katharine/pebble-stopwatch', 'Katharine', 'pebble-stopwatch') + + def test_plain_domain(self): + self.assert_parsed('github.com/user/repo', 'user', 'repo') + + def test_https_www_git_suffix_trailing_slash(self): + self.assert_parsed('https://www.github.com/user/repo.git/', 'user', 'repo') + + def test_ssh_form(self): + self.assert_parsed('git@github.com:user/repo.git', 'user', 'repo') + + def test_git_protocol(self): + self.assert_parsed('git://github.com/user/repo', 'user', 'repo') + + def test_dotted_names(self): + self.assert_parsed('github.com/user.name/repo.js', 'user.name', 'repo.js') + + def test_tree_with_branch(self): + self.assert_parsed('https://github.com/user/repo/tree/main', 'user', 'repo', 'tree', 'main') + + def test_tree_with_branch_and_subpath(self): + # The appstore "Remix" button shape. + self.assert_parsed( + 'github.com/emindeniz99/pebble-signals/tree/main/faces/slothvec', + 'emindeniz99', 'pebble-signals', 'tree', 'main/faces/slothvec') + + def test_blob_form(self): + self.assert_parsed( + 'https://github.com/user/repo/blob/main/src/app.js', + 'user', 'repo', 'blob', 'main/src/app.js') + + def test_commit_form(self): + self.assert_parsed('github.com/user/repo/commit/abc123', 'user', 'repo', 'commit', 'abc123') + + def test_query_and_fragment_stripped(self): + self.assert_parsed( + 'https://github.com/user/repo/tree/main/dir?tab=readme#l10', + 'user', 'repo', 'tree', 'main/dir') + + def test_empty_tree_tail_is_plain_repo(self): + self.assert_parsed('github.com/user/repo/tree/', 'user', 'repo') + + def test_rejects_other_github_pages(self): + for source in ('github.com/user/repo/releases', + 'github.com/user/repo/issues/12', + 'github.com/user/repo/pull/3', + 'github.com/user/repo/wiki'): + self.assertIsNone(parse_github_source(source), source) + + def test_case_insensitive_host(self): + self.assert_parsed('GitHub.com/user/repo', 'user', 'repo') + self.assert_parsed('HTTPS://WWW.GITHUB.COM/user/repo', 'user', 'repo') + + def test_percent_encoded_input(self): + self.assert_parsed('github.com/user/repo/tree/main/my%20dir', + 'user', 'repo', 'tree', 'main/my dir') + + def test_legacy_colon_form(self): + # The pre-parser server regex matched github.com[/:], so links using a + # bare colon after the domain must keep importing. + self.assert_parsed('github.com:user/repo', 'user', 'repo') + self.assert_parsed('https://github.com:user/repo.git', 'user', 'repo') + + def test_legacy_regex_parity(self): + # Every form the OLD import regex accepted (it truncated at the repo + # name) must still parse to the same user/project pair. + for source in ('https://github.com/user/repo', + 'http://github.com/user/repo', + 'www.github.com/user/repo', + 'git@github.com:user/repo', + 'git://github.com/user/repo', + 'github.com/user/repo.git', + 'github.com/user/repo/'): + self.assert_parsed(source, 'user', 'repo') + + def test_encoded_delimiters_inside_path_survive(self): + # %23/%3F are DATA; only raw '?' and '#' delimit. + self.assert_parsed('github.com/user/repo/tree/main/faces/foo%23bar', + 'user', 'repo', 'tree', 'main/faces/foo#bar') + self.assert_parsed('github.com/user/repo/tree/main/a%3Fb?tab=x#frag', + 'user', 'repo', 'tree', 'main/a?b') + + def test_rejects_non_github(self): + for source in ('gitlab.com/user/repo', 'https://example.com/user/repo', 'user', ''): + self.assertIsNone(parse_github_source(source), source) + + +class TestSplitRefAndPath(TestCase): + REFS = ['main', 'develop', 'feature/foo', 'feature/foo/bar', 'v1.0.0'] + + def test_plain_ref(self): + self.assertEqual(('main', ''), split_ref_and_path('main', self.REFS)) + + def test_ref_and_path(self): + self.assertEqual(('main', 'faces/slothvec'), + split_ref_and_path('main/faces/slothvec', self.REFS)) + + def test_slashed_ref_wins_over_short_ref(self): + self.assertEqual(('feature/foo', 'src'), + split_ref_and_path('feature/foo/src', self.REFS)) + + def test_longest_ref_wins(self): + self.assertEqual(('feature/foo/bar', 'src'), + split_ref_and_path('feature/foo/bar/src', self.REFS)) + + def test_tag_ref(self): + self.assertEqual(('v1.0.0', 'dir'), split_ref_and_path('v1.0.0/dir', self.REFS)) + + def test_unknown_ref_falls_back_to_first_segment(self): + self.assertEqual(('sha123', 'a/b'), split_ref_and_path('sha123/a/b', self.REFS)) + + def test_no_ref_list_falls_back_to_first_segment(self): + self.assertEqual(('main', 'faces/slothvec'), + split_ref_and_path('main/faces/slothvec', None)) + + +class TestSplitRefQualifier(TestCase): + def test_unqualified_passes_through(self): + self.assertEqual((('heads', 'tags'), 'main/faces'), + split_ref_qualifier('main/faces')) + + def test_refs_heads_pins_and_strips(self): + self.assertEqual((('heads',), 'feature/foo/src'), + split_ref_qualifier('refs/heads/feature/foo/src')) + + def test_refs_tags_pins_and_strips(self): + self.assertEqual((('tags',), 'v1.0.0/dir'), + split_ref_qualifier('refs/tags/v1.0.0/dir')) + + def test_strips_exactly_once(self): + # A branch literally named 'refs/heads/x' stays addressable through + # one extra qualifier — only the outermost is a spelling. + self.assertEqual((('heads',), 'refs/heads/x'), + split_ref_qualifier('refs/heads/refs/heads/x')) + + def test_composes_with_split_ref_and_path(self): + namespaces, refpath = split_ref_qualifier('refs/heads/feature/foo/src') + self.assertEqual(('feature/foo', 'src'), + split_ref_and_path(refpath, ['main', 'feature/foo'])) + + +class TestNormalizeSubpath(TestCase): + def test_empty_and_root(self): + self.assertEqual('', normalize_subpath('')) + self.assertEqual('', normalize_subpath(None)) + self.assertEqual('', normalize_subpath('/')) + self.assertEqual('', normalize_subpath('.')) + + def test_collapses(self): + self.assertEqual('faces/slothvec', normalize_subpath('/faces//slothvec/')) + self.assertEqual('a/b', normalize_subpath('a/./b')) + + def test_rejects_escapes(self): + # 'a/..' cancels out arithmetically, but '..' has no legitimate place + # in a GitHub tree URL — reject rather than normalize to the root. + for bad in ('..', '../x', 'a/../../b', 'a/..', 'a/../b'): + with self.assertRaises(ValueError): + normalize_subpath(bad) diff --git a/cloudpebble/ide/tests/test_import_archive.py b/cloudpebble/ide/tests/test_import_archive.py index e13e88a..ef9cc7b 100644 --- a/cloudpebble/ide/tests/test_import_archive.py +++ b/cloudpebble/ide/tests/test_import_archive.py @@ -344,3 +344,65 @@ def test_wipe_existing_rolls_back_on_failure(self): project = Project.objects.get(pk=self.project_id) self.assertEqual(project.source_files.count(), 1, "Original files should be preserved after failed import") + + +@mock.patch('ide.models.s3file.s3', fake_s3) +class TestImportArchiveWithRootHint(CloudpebbleTestCase): + """ root_hint imports one project out of a multi-project repository + archive — including repositories bigger than the whole-archive file + limit, where only the hinted subtree's size should matter. """ + + def setUp(self): + self.login() + + @staticmethod + def big_repo_bundle(filler_count, filler_dir='docs'): + spec = { + 'repo-main/package.json': make_package(package_options={'name': 'rootproject'}), + 'repo-main/src/main.c': '', + 'repo-main/faces/slothvec/package.json': make_package(package_options={'name': 'sloth'}), + 'repo-main/faces/slothvec/src/main.c': '', + } + for i in range(filler_count): + spec['repo-main/%s/filler-%d.txt' % (filler_dir, i)] = 'x' + return build_bundle(spec) + + def test_hint_picks_the_nested_project(self): + bundle = self.big_repo_bundle(0) + do_import_archive(self.project_id, bundle, root_hint='faces/slothvec') + project = Project.objects.get(pk=self.project_id) + self.assertEqual(project.app_short_name, 'sloth') + + def test_file_limit_applies_to_the_hinted_subtree_not_the_repo(self): + # 450 filler files: the whole archive is over the 400-entry limit, + # the hinted project is 2 files. + bundle = self.big_repo_bundle(450) + do_import_archive(self.project_id, bundle, root_hint='faces/slothvec') + project = Project.objects.get(pk=self.project_id) + self.assertEqual(project.app_short_name, 'sloth') + + def test_unhinted_big_archive_is_still_rejected(self): + bundle = self.big_repo_bundle(450) + with self.assertRaises(InvalidProjectArchiveException): + do_import_archive(self.project_id, bundle) + + def test_hinted_subtree_over_the_project_limit_is_rejected(self): + # The per-project limit still binds when the hinted project ITSELF + # is huge — the subtree filter must not quietly raise it to the + # scan ceiling. + bundle = self.big_repo_bundle(450, filler_dir='faces/slothvec/docs') + with self.assertRaisesRegex(InvalidProjectArchiveException, 'project directory'): + do_import_archive(self.project_id, bundle, root_hint='faces/slothvec') + + def test_hinted_scan_has_a_ceiling(self): + # The subtree limit must not turn the pre-scan into an unbounded + # walk over arbitrarily huge repositories. + bundle = self.big_repo_bundle(450) + with mock.patch('ide.tasks.archive.HINTED_ARCHIVE_SCAN_LIMIT', 100): + with self.assertRaises(InvalidProjectArchiveException): + do_import_archive(self.project_id, bundle, root_hint='faces/slothvec') + + def test_hint_with_no_project_there_fails(self): + bundle = self.big_repo_bundle(0) + with self.assertRaises(InvalidProjectArchiveException): + do_import_archive(self.project_id, bundle, root_hint='faces/missing') diff --git a/cloudpebble/ide/tests/test_run_github_link.py b/cloudpebble/ide/tests/test_run_github_link.py new file mode 100644 index 0000000..8792de3 --- /dev/null +++ b/cloudpebble/ide/tests/test_run_github_link.py @@ -0,0 +1,54 @@ +""" Tests for the appstore-source → import-deep-link translation in +ide.views.run. parse_github_source is the single authority on accepted +shapes; anything it rejects degrades to a plain repository link when a +user/repo pair is discernible, and to no link at all otherwise. """ + +from django.test import TestCase + +from ide.views.run import _github_import_url + + +class TestGithubImportUrl(TestCase): + def test_plain_repository(self): + self.assertEqual('/ide/import/github/user/repo', + _github_import_url('https://github.com/user/repo')) + + def test_legacy_colon_form(self): + self.assertEqual('/ide/import/github/user/repo', + _github_import_url('github.com:user/repo')) + + def test_tree_deep_link(self): + self.assertEqual('/ide/import/github/user/repo/tree/main/faces/slothvec', + _github_import_url('github.com/user/repo/tree/main/faces/slothvec')) + + def test_blob_deep_link(self): + self.assertEqual('/ide/import/github/user/repo/blob/main/src/app.js', + _github_import_url('https://github.com/user/repo/blob/main/src/app.js')) + + def test_unsafe_path_characters_are_reencoded(self): + # The parser percent-decodes; the deep link must re-encode so the + # href survives as one URL path. + self.assertEqual('/ide/import/github/user/repo/tree/main/my%20dir', + _github_import_url('github.com/user/repo/tree/main/my%20dir')) + self.assertEqual('/ide/import/github/user/repo/tree/main/foo%23bar', + _github_import_url('github.com/user/repo/tree/main/foo%23bar')) + + def test_ssh_port_form_makes_no_wrong_link(self): + # '22/user' must never be read as a user/repo pair. + self.assertEqual('', _github_import_url('ssh://git@github.com:22/user/repo')) + + def test_unrecognized_shapes_fall_back_to_the_repository(self): + for source in ('https://github.com/user/repo/Blob/main/x', + 'https://github.com/user/repo/releases', + 'https://github.com/user/repo.git/issues/5'): + self.assertEqual('/ide/import/github/user/repo', + _github_import_url(source), source) + + def test_url_inside_prose(self): + self.assertEqual( + '/ide/import/github/user/repo/tree/main/faces/x', + _github_import_url('Source: https://github.com/user/repo/tree/main/faces/x, enjoy!')) + + def test_no_github_no_link(self): + self.assertEqual('', _github_import_url('')) + self.assertEqual('', _github_import_url('https://gitlab.com/user/repo')) diff --git a/cloudpebble/ide/utils/github_urls.py b/cloudpebble/ide/utils/github_urls.py new file mode 100644 index 0000000..4c176d5 --- /dev/null +++ b/cloudpebble/ide/utils/github_urls.py @@ -0,0 +1,160 @@ +""" Parsing for GitHub project "sources" — the strings users paste into the +import dialog or arrive with via /ide/import/github/... deep links (e.g. the +appstore's "Remix on CloudPebble" button, which links to +.../import/github///tree//). + +This module is deliberately dependency-free (no Django, no PyGithub) so it can +be unit-tested standalone. Anything that needs the network — resolving which +prefix of an ambiguous "/" remainder is a real branch — lives in +ide.tasks.git and uses split_ref_and_path() from here with a real ref list. +""" +import posixpath +import re +from collections import namedtuple +from urllib.parse import unquote + +__author__ = 'emindeniz99' + +# A parsed source: +# user, project: the repository coordinates. +# kind: None (plain repo), 'tree', 'blob' or 'commit'. +# refpath: the raw remainder after /tree/ or /blob/ — "" or +# "/". GitHub branch names may contain slashes, so the split +# between ref and path can only be decided against the repository's real +# ref names (split_ref_and_path below). +GithubSource = namedtuple('GithubSource', ['user', 'project', 'kind', 'refpath']) + +# Accepted forms (raw query strings and fragments are stripped before +# percent-decoding, so encoded delimiters survive inside path segments): +# user/repo +# github.com/user/repo[.git][/] (bare, http, https, www; the +# legacy 'github.com:user/repo' +# colon form stays accepted) +# git@github.com:user/repo[.git] +# git://github.com/user/repo[.git] +# .../user/repo/tree/[/] +# .../user/repo/blob// +# .../user/repo/commit/ +# Anything else after the repo (releases, issues, wiki, ...) is rejected so a +# nonsensical import fails loudly instead of silently importing the repo root. +_SOURCE_RE = re.compile(r""" + ^ + (?: + (?i:(?:https?://)?(?:www\.)?github\.com)[/:] | + (?i:git@github\.com): | + (?i:git://github\.com)/ + )? + (?P[\w.-]+) + / + (?P[\w.-]+?) + (?:\.git)? + (?: + / + (?: + (?Ptree|blob|commit) + (?: + / + (?P.+?) + )? + )? + )? + /? + $ +""", re.VERBOSE) + + +def parse_github_source(source): + """ Parse a GitHub repository source string. + :param source: whatever the user gave us (URL, shorthand, deep-link tail). + :return: a GithubSource, or None if this isn't a recognizable GitHub source. + """ + # Split the RAW string on the query/fragment delimiters first, THEN + # percent-decode: an encoded '#' or '?' inside a path segment + # (…/tree/main/faces/foo%23bar) is data, not a delimiter. + raw = source.strip().split('#', 1)[0].split('?', 1)[0] + match = _SOURCE_RE.match(unquote(raw)) + if match is None: + return None + kind = match.group('kind') + refpath = match.group('refpath') + if refpath is not None: + refpath = refpath.strip('/') + if not refpath: + # "…/repo/tree/" and "…/repo/tree" carry no information — treat as the + # plain repository rather than rejecting the whole import. + kind, refpath = None, None + return GithubSource(match.group('user'), match.group('project'), kind, refpath) + + +def split_ref_qualifier(refpath): + """ Strip GitHub's self-qualified "refs/heads/..." / "refs/tags/..." + spelling (which its Raw button emits) off a /tree/ or /blob/ remainder. + + This is the single authority for the rule — the ref resolver and the + import API both need it, and a second copy could silently drift. + + :return: (namespaces, refpath) — the namespaces the URL pinned + (('heads',) or ('tags',), or both when unqualified) and the + remainder without the qualifier. + """ + parts = refpath.split('/') + if len(parts) >= 3 and parts[0] == 'refs' and parts[1] in ('heads', 'tags'): + return (parts[1],), '/'.join(parts[2:]) + return ('heads', 'tags'), refpath + + +def split_ref_and_path(refpath, ref_names): + """ Split a /tree/ or /blob/ remainder into (ref, path). + + GitHub branch names may contain slashes ("feature/foo"), so "a/b/c" is + ambiguous between ref "a/b" + path "c" and ref "a" + path "b/c". We match + the remainder against the repository's REAL ref names, longest prefix + first (the approach used by gitpick and gitingest; create-next-app instead + naive-splits and asks the user for --example-path in the ambiguous case). + Git's directory/file conflict rule means at most ONE branch and at most + ONE tag can prefix-match, so the split is deterministic — longest-first + only decides the cross-namespace case (tag "release" vs branch + "release/1.0") in favor of the more specific ref. + + Callers strip GitHub's self-qualified refs/heads|tags spelling first, + with split_ref_qualifier() — which also tells them which namespace the + URL pinned, something this function has no use for. + + :param refpath: "" or "/" (slashes and any refs/... + qualifier already stripped). + :param ref_names: iterable of the repository's ref names in the + namespaces the URL allows. Pass None when the list is unavailable — + the first segment is then taken as the ref, which is correct for + every ref without a slash in its name (commit SHAs included, since a + SHA is never slashed). + :return: (ref, path) — path is '' when the remainder is just a ref. + """ + parts = refpath.split('/') + if len(parts) == 1: + return refpath, '' + if ref_names is not None: + names = set(ref_names) + for i in range(len(parts), 0, -1): + candidate = '/'.join(parts[:i]) + if candidate in names: + return candidate, '/'.join(parts[i:]) + return parts[0], '/'.join(parts[1:]) + + +def normalize_subpath(path): + """ Normalize a repo-relative subdirectory: collapse separators, forbid + escapes. Returns '' for the repo root; raises ValueError on '..' or + absolute paths (deep links are attacker-suppliable). """ + if not path: + return '' + if '..' in path.split('/'): + # Reject '..' outright rather than letting cancelling segments + # normalize away — deep links are attacker-suppliable and there is no + # legitimate reason for a GitHub tree URL to contain '..'. + raise ValueError("Invalid project path: %r" % path) + normalized = posixpath.normpath(path.strip('/')) + if normalized in ('.', ''): + return '' + if normalized.startswith(('/', '..')): + raise ValueError("Invalid project path: %r" % path) + return normalized diff --git a/cloudpebble/ide/utils/project.py b/cloudpebble/ide/utils/project.py index d0f2a10..8601fe2 100644 --- a/cloudpebble/ide/utils/project.py +++ b/cloudpebble/ide/utils/project.py @@ -49,9 +49,31 @@ def is_manifest(kind, contents): return False -def find_project_root_and_manifest(project_items): +def _dir_matches_hint(base_dir, root_hint): + """ True if base_dir ('' or 'a/b/') is the root_hint directory, allowing + at most ONE wrapping folder above it ('-/...' in GitHub zips). + The wrapper limit matters: a bare suffix match would let a hint like + 'src' latch onto any directory of that name at any depth. """ + stripped = base_dir.rstrip('/') + if stripped == root_hint: + return True + if root_hint == '': + # An explicitly-selected repository root (e.g. a /blob// + # URL naming a root-level file): the only other acceptable location + # is directly inside the archive's single wrapping folder. + return '/' not in stripped + suffix = '/' + root_hint + return stripped.endswith(suffix) and '/' not in stripped[:-len(suffix)] + + +def find_project_root_and_manifest(project_items, root_hint=None): """ Given the contents of an archive, find a valid Pebble project. :param project_items: A list of BaseProjectItems + :param root_hint: If given, only accept a project whose directory is + root_hint, relative to the archive's own root (archives from GitHub + wrap everything in a '-/' folder, so the hint is matched + against the tail of the directory). Used by /tree// + imports to pick one project out of a repository that contains several. :return: A tuple of (path_to_project, manifest BaseProjectItem) """ SRC_DIR = 'src/' @@ -70,6 +92,11 @@ def find_project_root_and_manifest(project_items): continue # Ensure that the file is actually a manifest file if dir_end + len(name) == len(base_dir): + # A root hint pins the project to one directory; check it + # BEFORE reading, so the manifests a big repository keeps + # outside that directory are never opened or parsed. + if root_hint is not None and not _dir_matches_hint(base_dir[:dir_end], root_hint): + continue content = item.read() try: if is_manifest(name, content): @@ -105,8 +132,16 @@ def find_project_root_and_manifest(project_items): return base_dir, manifest_item # If we didn't find a valid project but we did find a broken manifest file, complain about it specifically. + # (Under a hint, manifests are only ever read inside the hinted directory, + # so a broken one is always the actionable diagnosis.) if invalid_package_path: raise InvalidProjectArchiveException(_("The file %s does not contain a valid JSON object." % invalid_package_path)) + if root_hint: + raise InvalidProjectArchiveException( + _("No valid Pebble project found at '%s' in this repository.") % root_hint) + if root_hint is not None: + raise InvalidProjectArchiveException( + _("No valid Pebble project found at the root of this repository.")) else: raise InvalidProjectArchiveException( _( diff --git a/cloudpebble/ide/views/run.py b/cloudpebble/ide/views/run.py index 686ae1e..63a7c59 100644 --- a/cloudpebble/ide/views/run.py +++ b/cloudpebble/ide/views/run.py @@ -1,6 +1,7 @@ import json import logging import re +from urllib.parse import quote as http_quote import requests as http_requests from django.conf import settings @@ -10,6 +11,8 @@ from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie from django.views.decorators.http import require_safe +from ide.utils.github_urls import parse_github_source + logger = logging.getLogger(__name__) PLATFORM_DISPLAY_NAMES = { @@ -58,6 +61,31 @@ def _get_pbw_url(app_info): return '%s%s' % (settings.APPSTORE_API_BASE, pbw_file) +def _github_import_url(source): + """Best import deep link for an appstore 'source' field, or ''. + + The field may be prose, so the token containing github.com is picked out + and handed to the import URL parser — the single authority on which + shapes the importer accepts (tree/blob/commit deep links included). + Anything it rejects (ports, /releases/, unrecognized casing) degrades to + a plain repository link when a user/repo pair is discernible, rather + than a deep link that would fail to import.""" + if not source or 'github.com' not in source.lower(): + return '' + token = next((tok.strip('\'"<>(),.;:') for tok in source.split() + if 'github.com' in tok.lower()), '') + parsed = parse_github_source(token) + if parsed: + pieces = [parsed.user, parsed.project] + if parsed.kind: + pieces += [parsed.kind, parsed.refpath] + return '/ide/import/github/%s' % http_quote('/'.join(pieces), safe='/') + gh_match = re.search(r"github\.com/([\w.-]+/[\w.-]+)", token) + if gh_match: + return '/ide/import/github/%s' % gh_match.group(1).removesuffix('.git') + return '' + + @require_safe @ensure_csrf_cookie def run_app(request, app_id): @@ -99,14 +127,7 @@ def run_app(request, app_id): app_hearts = app_info.get('hearts', 0) app_uuid = app_info.get('uuid', '') - # Check if source is a GitHub URL - source = app_info.get('source') or '' - github_import_url = '' - if source and 'github.com/' in source: - # Extract account/project from URL like https://github.com/user/repo - gh_match = re.search(r'github\.com/([^/]+/[^/]+)', source) - if gh_match: - github_import_url = '/ide/import/github/%s' % gh_match.group(1) + github_import_url = _github_import_url(app_info.get('source') or '') return render(request, 'ide/run.html', { 'app_id': app_id,