Merge pull request #597 from coreos-inc/tag-validation

Update tag validation

endpoints/api/tag.py

@@ -8,6 +8,7 @@ from endpoints.api import (resource, nickname, require_repo_read, require_repo_w
 from endpoints.api.image import image_view
 from data import model
 from auth.auth_context import get_authenticated_user
+from util.names import TAG_ERROR, TAG_REGEX
 
 
 @resource('/v1/repository/<repopath:repository>/tag/')
@@ -85,6 +86,10 @@ class RepositoryTag(RepositoryParamResource):
   @validate_json_request('MoveTag')
   def put(self, namespace, repository, tag):
     """ Change which image a tag points to or create a new tag."""
+
+    if not TAG_REGEX.match(tag):
+      abort(400, TAG_ERROR)
+
     image_id = request.get_json()['image']
     image = model.image.get_repo_image(namespace, repository, image_id)
     if not image:

endpoints/v1/tag.py

@@ -5,7 +5,7 @@ import json
 from flask import abort, request, jsonify, make_response, session
 
 from app import app
-from util.names import parse_repository_name
+from util.names import TAG_ERROR, TAG_REGEX, parse_repository_name
 from auth.auth import process_auth
 from auth.permissions import (ReadRepositoryPermission,
                               ModifyRepositoryPermission)
@@ -56,6 +56,9 @@ def put_tag(namespace, repository, tag):
   permission = ModifyRepositoryPermission(namespace, repository)
 
   if permission.can():
+    if not TAG_REGEX.match(tag):
+      abort(400, TAG_ERROR)
+
     docker_image_id = json.loads(request.data)
     model.tag.create_or_update_tag(namespace, repository, tag, docker_image_id)
 

static/directives/tag-operations-dialog.html

@@ -34,7 +34,7 @@
             <div ng-show="!addingTag">
               <input type="text" class="form-control" id="tagName"
                      placeholder="Enter tag name"
-                     ng-model="tagToCreate" ng-pattern="/^([a-z0-9_\.-]){3,30}$/"
+                     ng-model="tagToCreate" ng-pattern="/^[\w][\w\.-]{0,127}$/"
                      ng-disabled="creatingTag" autofocus required>
 
               <div style="margin: 10px; margin-top: 20px;"
@@ -121,4 +121,4 @@
     <span class="label label-default tag">{{ revertTagInfo.tag.name }}</span> to image
     <span class="image-id">{{ revertTagInfo.image_id.substr(0, 12) }}?</span>
   </div>
-</div>
+</div>

test/registry_tests.py

@@ -212,8 +212,7 @@ class RegistryTestCase(LiveServerTestCase):
 
 
     # PUT /v1/repositories/{namespace}/{repository}/tags/latest
-    self.conduct('PUT', '/v1/repositories/%s/%s/tags/latest' % (namespace, repository),
-                 data='"' + images[0]['id'] + '"')
+    self.do_tag(namespace, repository, 'latest', images[0]['id'])
 
     # PUT /v1/repositories/{namespace}/{repository}/images
     self.conduct('PUT', '/v1/repositories/%s/%s/images' % (namespace, repository),
@@ -246,6 +245,10 @@ class RegistryTestCase(LiveServerTestCase):
       self.conduct('GET', image_prefix + 'json')
       self.conduct('GET', image_prefix + 'layer')
 
+  def do_tag(self, namespace, repository, tag, image_id, expected_code=200):
+    self.conduct('PUT', '/v1/repositories/%s/%s/tags/%s' % (namespace, repository, tag),
+                 data='"%s"' % image_id, expected_code=expected_code)
+
   def conduct_api_login(self, username, password):
     self.conduct('POST', '/api/v1/signin',
                  data=json.dumps(dict(username=username, password=password)),
@@ -437,5 +440,19 @@ class RegistryTests(RegistryTestCase):
     # org.
     self.do_pull('buynlarge', 'newrepo', 'devtable', 'password')
 
+  def test_tag_validation(self):
+    image_id = 'onlyimagehere'
+    images = [{
+      'id': image_id
+    }]
+    self.do_push('public', 'newrepo', 'public', 'password', images)
+    self.do_tag('public', 'newrepo', '1', image_id)
+    self.do_tag('public', 'newrepo', 'x' * 128, image_id)
+    self.do_tag('public', 'newrepo', '', image_id, expected_code=400)
+    self.do_tag('public', 'newrepo', 'x' * 129, image_id, expected_code=400)
+    self.do_tag('public', 'newrepo', '.fail', image_id, expected_code=400)
+    self.do_tag('public', 'newrepo', '-fail', image_id, expected_code=400)
+
+
 if __name__ == '__main__':
   unittest.main()

test/test_api_usage.py

@@ -2164,6 +2164,12 @@ class TestListAndDeleteTag(ApiTestCase):
 
     self.assertEquals(staging_images, json['images'])
 
+    # Require a valid tag name.
+    self.putResponse(RepositoryTag,
+                     params=dict(repository=ADMIN_ACCESS_USER + '/complex', tag='-fail'),
+                     data=dict(image=staging_images[0]['id']),
+                     expected_code=400)
+
     # Add a new tag to the staging image.
     self.putResponse(RepositoryTag,
                      params=dict(repository=ADMIN_ACCESS_USER + '/complex', tag='sometag'),

util/names.py

@@ -6,6 +6,10 @@ from uuid import uuid4
 
 REPOSITORY_NAME_REGEX = re.compile(r'^[\.a-zA-Z0-9_-]+$')
 
+TAG_REGEX = re.compile(r'^[\w][\w\.-]{0,127}$')
+TAG_ERROR = ('Invalid tag: must match [A-Za-z0-9_.-], NOT start with "." or "-", '
+             'and can contain 1-128 characters')
+
 def parse_namespace_repository(repository, include_tag=False):
   parts = repository.rstrip('/').split('/', 1)
   if len(parts) < 2: