Skip to content
Draft
Show file tree
Hide file tree
Changes from 7 commits
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
53 changes: 45 additions & 8 deletions cloudpebble/ide/api/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1029,14 +1030,49 @@ 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."))

Comment thread
emindeniz99 marked this conversation as resolved.
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/<x> is unambiguously a branch or tag — exactly
# equivalent to typing it into the branch box, so every existing flow
# (add_remote included) stays available.
branch, refpath, kind = refpath, None, None
Comment thread
emindeniz99 marked this conversation as resolved.
elif refpath:
Comment thread
emindeniz99 marked this conversation as resolved.
# 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)
Expand All @@ -1045,10 +1081,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}


Expand Down
179 changes: 179 additions & 0 deletions cloudpebble/ide/static/ide/js/__tests__/github-import-prefill.test.js
Original file line number Diff line number Diff line change
@@ -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/<branch>/<subdir> 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 /<user>/<repo>/<branch> 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');
});
});
35 changes: 27 additions & 8 deletions cloudpebble/ide/static/ide/js/project_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/<ref>/<subdir> 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).
Comment thread
emindeniz99 marked this conversation as resolved.
disable_import_controls();
active_set.find('.progress').removeClass('hide');
do_import(Ajax.Post('/ide/import/github', {
Expand Down Expand Up @@ -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/<user>/<repo>/tree/<branch>/<subdir>). 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');
}
Expand Down
21 changes: 18 additions & 3 deletions cloudpebble/ide/tasks/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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('/')
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Comment thread
emindeniz99 marked this conversation as resolved.
Comment thread
emindeniz99 marked this conversation as resolved.
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):
Expand Down
Loading