fix(environments) Disallow `/` in future environment names
diff --git a/src/sentry/api/endpoints/project_releases.py b/src/sentry/api/endpoints/project_releases.py
index 95561f38b0b59..cb38d0b8c4a10 100644
--- a/src/sentry/api/endpoints/project_releases.py
+++ b/src/sentry/api/endpoints/project_releases.py
@@ -16,8 +16,6 @@
 from sentry.constants import VERSION_LENGTH
 from sentry.signals import release_created
 
-BAD_RELEASE_CHARS = '\n\f\t/'
-
 
 class ReleaseSerializer(serializers.Serializer):
     version = serializers.CharField(max_length=VERSION_LENGTH, required=True)
diff --git a/src/sentry/api/endpoints/release_deploys.py b/src/sentry/api/endpoints/release_deploys.py
index a12e425af26a5..0c2e0a2a2f3ec 100644
--- a/src/sentry/api/endpoints/release_deploys.py
+++ b/src/sentry/api/endpoints/release_deploys.py
@@ -23,6 +23,12 @@ class DeploySerializer(serializers.Serializer):
     dateStarted = serializers.DateTimeField(required=False)
     dateFinished = serializers.DateTimeField(required=False)
 
+    def validate_environment(self, attrs, source):
+        value = attrs[source]
+        if not Environment.is_valid_name(value):
+            raise serializers.ValidationError('Invalid value for environment')
+        return attrs
+
 
 class ReleaseDeploysEndpoint(OrganizationReleasesBaseEndpoint):
     doc_section = DocSection.RELEASES
diff --git a/src/sentry/constants.py b/src/sentry/constants.py
index f99a5940c369a..17fda7ae67a98 100644
--- a/src/sentry/constants.py
+++ b/src/sentry/constants.py
@@ -76,6 +76,9 @@ def get_all_languages():
 MAX_CULPRIT_LENGTH = 200
 MAX_EMAIL_FIELD_LENGTH = 75
 
+ENVIRONMENT_NAME_PATTERN = r'^[^\n\r\f\/]*$'
+ENVIRONMENT_NAME_MAX_LENGTH = 64
+
 # Team slugs which may not be used. Generally these are top level URL patterns
 # which we don't want to worry about conflicts on.
 RESERVED_ORGANIZATION_SLUGS = frozenset(
diff --git a/src/sentry/interfaces/schemas.py b/src/sentry/interfaces/schemas.py
index e437739b2e6c2..6f19fe4beb8a2 100644
--- a/src/sentry/interfaces/schemas.py
+++ b/src/sentry/interfaces/schemas.py
@@ -1,6 +1,6 @@
 """
 sentry.interfaces.schemas
-~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~
 
 :copyright: (c) 2010-2017 by the Sentry Team, see AUTHORS for more details.
 :license: BSD, see LICENSE for more details.
@@ -20,6 +20,8 @@
     MAX_TAG_KEY_LENGTH,
     MAX_TAG_VALUE_LENGTH,
     VALID_PLATFORMS,
+    ENVIRONMENT_NAME_MAX_LENGTH,
+    ENVIRONMENT_NAME_PATTERN,
 )
 from sentry.interfaces.base import InterfaceValidationError
 from sentry.models import EventError
@@ -417,7 +419,8 @@ def apierror(message="Invalid data"):
         },
         'environment': {
             'type': 'string',
-            'maxLength': 64,
+            'maxLength': ENVIRONMENT_NAME_MAX_LENGTH,
+            'pattern': ENVIRONMENT_NAME_PATTERN,
         },
         'modules': {'type': 'object'},
         'extra': {'type': 'object'},
@@ -781,7 +784,6 @@ def validate_and_default_interface(data, interface, name=None,
                     default = schema['properties'][p]['default']
                     data[p] = default() if callable(default) else default
                 else:
-                    # TODO raise as shortcut?
                     errors.append({'type': EventError.MISSING_ATTRIBUTE, 'name': p})
 
     validator_errors = list(validator.iter_errors(data))
@@ -793,7 +795,12 @@ def validate_and_default_interface(data, interface, name=None,
     for key, group in groupby(keyed_errors, lambda e: e.path[0]):
         ve = six.next(group)
         is_max = ve.validator.startswith('max')
-        error_type = EventError.VALUE_TOO_LONG if is_max else EventError.INVALID_DATA
+        if is_max:
+            error_type = EventError.VALUE_TOO_LONG
+        elif key == 'environment':
+            error_type = EventError.INVALID_ENVIRONMENT
+        else:
+            error_type = EventError.INVALID_DATA
         errors.append({'type': error_type, 'name': name or key, 'value': data[key]})
 
         if 'default' in ve.schema:
diff --git a/src/sentry/models/environment.py b/src/sentry/models/environment.py
index a0843bfe616f2..01b89ec54e160 100644
--- a/src/sentry/models/environment.py
+++ b/src/sentry/models/environment.py
@@ -1,6 +1,6 @@
 """
-sentry.models.release
-~~~~~~~~~~~~~~~~~~~~~
+sentry.models.environment
+~~~~~~~~~~~~~~~~~~~~~~~~~
 
 :copyright: (c) 2010-2014 by the Sentry Team, see AUTHORS for more details.
 :license: BSD, see LICENSE for more details.
@@ -10,9 +10,16 @@
 from django.db import IntegrityError, models, transaction
 from django.utils import timezone
 
+from sentry.constants import (
+    ENVIRONMENT_NAME_PATTERN,
+    ENVIRONMENT_NAME_MAX_LENGTH
+)
 from sentry.db.models import (BoundedPositiveIntegerField, FlexibleForeignKey, Model, sane_repr)
 from sentry.utils.cache import cache
 from sentry.utils.hashlib import md5_text
+import re
+
+OK_NAME_PATTERN = re.compile(ENVIRONMENT_NAME_PATTERN)
 
 
 class EnvironmentProject(Model):
@@ -45,6 +52,17 @@ class Meta:
 
     __repr__ = sane_repr('organization_id', 'name')
 
+    @classmethod
+    def is_valid_name(cls, value):
+        """Limit length and reject problematic bytes
+
+        If you change the rules here also update the event ingestion schema
+        in sentry.interfaces.schemas
+        """
+        if len(value) > ENVIRONMENT_NAME_MAX_LENGTH:
+            return False
+        return OK_NAME_PATTERN.match(value) is not None
+
     @classmethod
     def get_cache_key(cls, organization_id, name):
         return 'env:2:%s:%s' % (organization_id, md5_text(name).hexdigest())
diff --git a/src/sentry/models/eventerror.py b/src/sentry/models/eventerror.py
index ac532e5c2b398..ed79b1304f94f 100644
--- a/src/sentry/models/eventerror.py
+++ b/src/sentry/models/eventerror.py
@@ -28,6 +28,7 @@ class EventError(object):
     RESTRICTED_IP = 'restricted_ip'
     FUTURE_TIMESTAMP = 'future_timestamp'
     PAST_TIMESTAMP = 'past_timestamp'
+    INVALID_ENVIRONMENT = 'invalid_environment'
 
     JS_GENERIC_FETCH_ERROR = 'js_generic_fetch_error'  # deprecated in favor of FETCH_GENERIC_ERROR
     FETCH_GENERIC_ERROR = 'fetch_generic_error'
@@ -67,6 +68,7 @@ class EventError(object):
         RESTRICTED_IP: u'Cannot fetch resource due to restricted IP address on {url}',
         FUTURE_TIMESTAMP: u'Invalid timestamp (in future)',
         PAST_TIMESTAMP: u'Invalid timestamp (too old)',
+        INVALID_ENVIRONMENT: u'Environment cannot contain / or newlines.',
         # deprecated in favor of FETCH_GENERIC_ERROR
         JS_GENERIC_FETCH_ERROR: u'Unable to fetch resource: {url}',
         FETCH_GENERIC_ERROR: u'Unable to fetch resource: {url}',
diff --git a/tests/sentry/api/endpoints/test_release_deploys.py b/tests/sentry/api/endpoints/test_release_deploys.py
index ee27f249f6ec3..d759531d505c6 100644
--- a/tests/sentry/api/endpoints/test_release_deploys.py
+++ b/tests/sentry/api/endpoints/test_release_deploys.py
@@ -111,3 +111,32 @@ def test_simple(self):
             project=project, release=release, environment=environment
         )
         assert rpe.last_deploy_id == deploy.id
+
+    def test_environment_validation_failure(self):
+        project = self.create_project(name='example')
+        release = Release.objects.create(
+            organization_id=project.organization_id,
+            version='123',
+            total_deploys=0,
+        )
+        release.add_project(project)
+
+        url = reverse(
+            'sentry-api-0-organization-release-deploys',
+            kwargs={
+                'organization_slug': project.organization.slug,
+                'version': release.version,
+            }
+        )
+
+        self.login_as(user=self.user)
+        response = self.client.post(
+            url,
+            data={
+                'name': 'foo',
+                'environment': 'bad/name',
+                'url': 'https://www.example.com',
+            }
+        )
+        assert response.status_code == 400, response.content
+        assert 0 == Deploy.objects.count()
diff --git a/tests/sentry/coreapi/tests.py b/tests/sentry/coreapi/tests.py
index 214508849b528..013f5287c7e6d 100644
--- a/tests/sentry/coreapi/tests.py
+++ b/tests/sentry/coreapi/tests.py
@@ -441,6 +441,16 @@ def test_environment_too_long(self):
         assert data['errors'][0]['name'] == 'environment'
         assert data['errors'][0]['value'] == 'a' * 65
 
+    def test_environment_invalid(self):
+        data = self.validate_and_normalize({
+            'environment': 'a/b',
+        })
+        assert not data.get('environment')
+        assert len(data['errors']) == 1
+        assert data['errors'][0]['type'] == 'invalid_environment'
+        assert data['errors'][0]['name'] == 'environment'
+        assert data['errors'][0]['value'] == 'a/b'
+
     def test_environment_as_non_string(self):
         data = self.validate_and_normalize({
             'environment': 42,
diff --git a/tests/sentry/models/test_environment.py b/tests/sentry/models/test_environment.py
index fab8e18d41f3b..c69775129cb1e 100644
--- a/tests/sentry/models/test_environment.py
+++ b/tests/sentry/models/test_environment.py
@@ -36,3 +36,20 @@ def test_simple(self):
                 project.organization_id,
                 'prod',
             ).id == env.id
+
+
+@pytest.mark.parametrize('val,expected', [
+    ('42', True),
+    ('ok', True),
+    ('production', True),
+    ('deadbeef', True),
+    ('staging.0.1.company', True),
+    ('valid_under', True),
+    ('spaces ok', True),
+    ('no/slashes', False),
+    ('no\nnewlines', False),
+    ('no\rcarriage', False),
+    ('no\fform-feed', False),
+])
+def test_valid_name(val, expected):
+    assert Environment.is_valid_name(val) == expected
diff --git a/tests/sentry/test_event_manager.py b/tests/sentry/test_event_manager.py
index 0b526ae3bcbba..a85696623285c 100644
--- a/tests/sentry/test_event_manager.py
+++ b/tests/sentry/test_event_manager.py
@@ -955,6 +955,14 @@ def test_environment(self):
 
         assert dict(event.tags).get('environment') == 'beta'
 
+    def test_invalid_environment(self):
+        manager = EventManager(self.make_event(**{
+            'environment': 'bad/name',
+        }))
+        manager.normalize()
+        event = manager.save(self.project.id)
+        assert dict(event.tags).get('environment') is None
+
     @mock.patch('sentry.event_manager.eventstream.insert')
     def test_group_environment(self, eventstream_insert):
         release_version = '1.0'

While we can't reasonably change existing data, we can prevent more in the future. Having `/` in an environment name makes it impossible to hide the environment.

Refs APP-323
Refs ISSUE-24