-
Notifications
You must be signed in to change notification settings - Fork 0
Django 프로젝트 설정 값에 환경변수를 사용하여 보안 강화 #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e9c8a3d
19c0bd6
9866419
8d2a53f
0a9a9da
98648fa
1eb40ef
7557506
8990a0f
d8b0d23
fa942c0
3db6249
c9d5023
902b3fe
3ebafc3
d84c9cb
b0a569f
6f0b39b
618f3bb
59b02e3
3a7259f
9107090
a8a2a5e
d7f68b9
2843ba3
3e8a6ad
aab2a3c
08174f2
3c189ff
48e28d8
70c9545
2b52476
70ee7f3
e6fde3b
2b95446
f62251b
af50d2a
0ca9cdc
fff363d
98314ea
a8b1cb6
86399eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # Sample .env file for development and testing purposes | ||
|
|
||
| # Set DEBUG to true for development, false for production | ||
| DEBUG=false | ||
|
|
||
| # Django secret key for cryptographic signing | ||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # NOTE: Python handles forward slashes correctly on all platforms, including Windows. | ||
| SECRET_KEY_FILE=../.secrets/secret_key.txt | ||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # SECRET_KEY='<YOUR_SECRET_KEY>' | ||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # Allowed hosts for the Django application | ||
| # ALLOWED_HOSTS=["example.com", "www.example.com"] | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,223 @@ | ||||||
| """ | ||||||
| 환경 변수와 관련된 기능 혹은 유틸리티 모음. | ||||||
| """ | ||||||
|
|
||||||
| import json | ||||||
| import os | ||||||
| from pathlib import Path | ||||||
| from typing import Any, Optional, Union | ||||||
|
|
||||||
| import dotenv | ||||||
|
|
||||||
| # Define truthy and falsy values | ||||||
| _TRUTHY_VALUES = ('true', '1', 't', 'y', 'yes', 'on') | ||||||
| _FALSY_VALUES = ('false', '0', 'f', 'n', 'no', 'off') | ||||||
|
|
||||||
|
|
||||||
| def load(dotenv_path: Optional[Path] = None): | ||||||
| """.env 파일이 있다면 .env 파일에서 환경 변수를 로드합니다. | ||||||
| Args: | ||||||
| dotenv_path: .env 파일 경로 (기본값: None) | ||||||
| """ | ||||||
| # Load .env file if it exists | ||||||
| if (dotenv_path is not None) and dotenv_path.exists(): | ||||||
| dotenv.load_dotenv(dotenv_path=dotenv_path, override=True) | ||||||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
|
|
||||||
| def get(key: str, default: Optional[str] = None, required: bool = False, strip: bool = True, blank: bool = False) -> Optional[str]: | ||||||
| """환경 변수 값을 가져옵니다. | ||||||
| Args: | ||||||
| key: 환경 변수 이름 | ||||||
| default: 기본값 (환경 변수가 없을 때 반환) | ||||||
| required: 필수 여부 (True일 때 환경 변수가 없으면 예외 발생) | ||||||
| strip: 값의 앞뒤 공백 제거 여부 (기본값: True) | ||||||
| blank: 빈 문자열 허용 여부 (False일 때 빈 문자열은 None으로 간주) | ||||||
| Returns: | ||||||
| 환경 변수 값 또는 기본값 | ||||||
| Raises: | ||||||
| ValueError: required=True이고 환경 변수가 설정되지 않은 경우 | ||||||
| """ | ||||||
| string_value = os.getenv(key, default) | ||||||
|
|
||||||
| if strip and (string_value is not None): | ||||||
| string_value = string_value.strip() | ||||||
|
|
||||||
| if not blank and (string_value == ''): | ||||||
| string_value = None | ||||||
|
|
||||||
| if required and (string_value is None): | ||||||
| raise ValueError( | ||||||
| f'Environment variable "{key}" is required but not set.' | ||||||
| ) | ||||||
|
|
||||||
| return string_value | ||||||
|
|
||||||
|
|
||||||
| def get_bool(key: str, default: Optional[bool] = None, strip: bool = True, required: bool = False) -> Optional[bool]: | ||||||
| """환경 변수를 불린 값으로 파싱합니다. | ||||||
| Args: | ||||||
| key: 환경 변수 이름 | ||||||
| default: 기본값 (환경 변수가 없을 때 반환) | ||||||
| strip: 앞뒤 공백 제거 여부 (기본값: True) | ||||||
| required: 필수 여부 (True일 때 환경 변수가 없으면 예외 발생) | ||||||
| Returns: | ||||||
| 불린 값 또는 기본값 | ||||||
| Raises: | ||||||
| ValueError: 잘못된 불린 값이거나 required=True이고 환경 변수가 설정되지 않은 경우 | ||||||
| """ | ||||||
| raw_value = os.getenv(key) | ||||||
| boolean_value = default | ||||||
|
|
||||||
| if raw_value is not None: | ||||||
| if _is_truthy(raw_value, strip=strip): | ||||||
| boolean_value = True | ||||||
| elif _is_falsy(raw_value, strip=strip): | ||||||
| boolean_value = False | ||||||
| else: | ||||||
| raise ValueError( | ||||||
| f'Environment variable "{key}" has invalid boolean value.' | ||||||
| ) | ||||||
|
|
||||||
| if required and (boolean_value is None): | ||||||
| raise ValueError( | ||||||
| f'Environment variable "{key}" is required but not set.' | ||||||
| ) | ||||||
|
|
||||||
| return boolean_value | ||||||
|
|
||||||
|
|
||||||
| def get_json(key: str, default: Optional[Any] = None, required: bool = False) -> Optional[Any]: | ||||||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| """환경 변수 값을 JSON으로 파싱하여 반환하거나 기본값을 반환합니다. | ||||||
| Args: | ||||||
| key: 환경 변수 이름 | ||||||
| default: 기본값 (환경 변수가 없을 때 반환) | ||||||
| required: 필수 여부 (True일 때 환경 변수가 없으면 예외 발생) | ||||||
| Returns: | ||||||
| JSON으로 파싱된 값 또는 기본값 | ||||||
| Raises: | ||||||
| ValueError: 환경 변수 값이 올바른 JSON이 아니거나 required=True이고 환경 변수가 설정되지 않은 경우 | ||||||
| """ | ||||||
| raw_value = os.getenv(key) | ||||||
|
||||||
| json_value = default | ||||||
|
|
||||||
| if raw_value is not None: | ||||||
| try: | ||||||
| json_value = json.loads(raw_value) | ||||||
| except ValueError as e: | ||||||
|
||||||
| except ValueError as e: | |
| except json.JSONDecodeError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 예외처리 분기의 증가를 막고자 의도적으로 JSONDecodeError의 상위 클래스인 ValueError로 예외처리 한 것이므로 유지하겠습니다.
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message is vague and doesn't specify what went wrong with the JSON parsing. The original exception message from json.loads() contains valuable debugging information. Consider including it in the error message or using str(e) to provide more context.
Suggested improvement:
raise ValueError(
f'Environment variable "{key}" has invalid JSON value: {str(e)}'
) from e| f'Environment variable "{key}" has invalid JSON value.' | |
| f'Environment variable "{key}" has invalid JSON value: {str(e)}' |
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message is vague and doesn't provide information about what made the file path invalid. The original OSError contains the specific reason (e.g., permission denied, path too long, invalid characters). Consider including the original error message.
Suggested improvement:
raise ValueError(
f'Environment variable "{key}" has invalid file path: {str(e)}'
) from e| f'Environment variable "{key}" has invalid file path.' | |
| f'Environment variable "{key}" has invalid file path: {str(e)}' |
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The get_path function lacks validation to prevent potential security issues with file permissions. When reading from SECRET_KEY_FILE, there's no check to ensure the file has appropriate permissions (e.g., not world-readable). Consider adding a permission check in get_path or documenting that callers should verify file permissions when dealing with sensitive data.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,20 +12,59 @@ | |||||||||
|
|
||||||||||
| from pathlib import Path | ||||||||||
|
|
||||||||||
| from app import env | ||||||||||
|
|
||||||||||
| # Build paths inside the project like this: BASE_DIR / 'subdir'. | ||||||||||
| BASE_DIR = Path(__file__).resolve().parent.parent | ||||||||||
|
|
||||||||||
|
|
||||||||||
| env.load(dotenv_path=BASE_DIR / '.env') | ||||||||||
|
|
||||||||||
|
|
||||||||||
| # Quick-start development settings - unsuitable for production | ||||||||||
| # See https://docs.djangoproject.com/en/5.2/howto/deployment/checklist/ | ||||||||||
|
|
||||||||||
| # SECURITY WARNING: keep the secret key used in production secret! | ||||||||||
| SECRET_KEY = 'django-insecure-^&757(fcj_5idk9lfj5%mc#5ozvfoigka&#z73lxc_(!4j_e$o' | ||||||||||
| secret_key = env.get("SECRET_KEY") | ||||||||||
| secret_key_file = env.get_path("SECRET_KEY_FILE") | ||||||||||
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| # NOTE: path traversal attack에 대하여, | ||||||||||
| # Docker Compose를 구성할 경우, secret이 담긴 파일이 프로젝트 디렉터리 외부에 있는 경우도 있다. | ||||||||||
| # 따라서 relative_to 를 설정할 경우, "/run/secrets/*" 와 같은 경로에 접근을 못하게 될 가능성이 높다. | ||||||||||
| # 더군다나 위 코드는 최초 설정시에만 실행되기에 약점의 크기가 상당히 작다고 판단하여 | ||||||||||
| # path traversal attack을 허용할 여지가 있더라도, 더 좋은 방법을 찾기 전까지는 relative_to를 설정하지 않는다. | ||||||||||
|
|
||||||||||
| if secret_key and secret_key_file: | ||||||||||
| raise ValueError("Cannot set both SECRET_KEY and SECRET_KEY_FILE") | ||||||||||
| elif secret_key: | ||||||||||
| SECRET_KEY = secret_key | ||||||||||
|
Comment on lines
+38
to
+39
|
||||||||||
| elif secret_key_file: | ||||||||||
| # NOTE: 명확한 에러 추적을 위해 파일 경로는 오류 메시지로 노출. | ||||||||||
| # 오류가 발생하면 애플리케이션 구동이 안되기에, 보안을 위해 키 값도 아닌, | ||||||||||
| # 경로 자체를 숨기는 것은 과하다고 판단함. | ||||||||||
| if not secret_key_file.exists(): | ||||||||||
| raise ValueError(f"SECRET_KEY_FILE does not exist: {secret_key_file}") | ||||||||||
| if not secret_key_file.is_file(): | ||||||||||
| raise ValueError(f"SECRET_KEY_FILE is not a file: {secret_key_file}") | ||||||||||
|
|
||||||||||
|
||||||||||
| if secret_key_file.stat().st_size > 1024: | |
| raise ValueError(f"SECRET_KEY_FILE is too large: {secret_key_file}") |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SECRET_KEY file is read without size validation, which could lead to memory issues if an attacker or misconfiguration points to a very large file. Consider adding a file size check before reading (e.g., if secret_key_file.stat().st_size > MAX_SECRET_KEY_SIZE: raise ValueError(...)) to prevent potential denial of service.
hepheir marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential security issue: In production (when DEBUG=False), if the ALLOWED_HOSTS environment variable is not set, the application will crash with a ValueError. However, there's no validation that the provided ALLOWED_HOSTS is a non-empty list. An empty JSON array [] would pass the required=True check but would make the application reject all requests. Consider adding validation to ensure ALLOWED_HOSTS is not empty in production: if not DEBUG and not ALLOWED_HOSTS: raise ValueError("ALLOWED_HOSTS cannot be empty in production")
| ALLOWED_HOSTS = env.get_json('ALLOWED_HOSTS', required=True) | |
| ALLOWED_HOSTS = env.get_json('ALLOWED_HOSTS', required=True) | |
| if not ALLOWED_HOSTS: | |
| raise ValueError("ALLOWED_HOSTS cannot be empty in production") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#4 (comment) 반복하여 제안과 번복되고 있는 내용.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sample shows
DEBUG=falsebut the comment says "Set DEBUG to true for development". This is confusing - a sample file for development should probably haveDEBUG=trueby default, or the comment should clarify why false is recommended even for development.