-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): close credential persistence gaps #232
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
8be6cc1
0985e6d
8673582
cef66e6
18fe6a4
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 |
|---|---|---|
|
|
@@ -15,43 +15,75 @@ | |
| "access_token", | ||
| "api_key", | ||
| "apikey", | ||
| "auth_token", | ||
| "authorization", | ||
| "client_secret", | ||
| "credential", | ||
| "password", | ||
| "private_key", | ||
| "refresh_token", | ||
| "github_token", | ||
| "secret", | ||
| "token", | ||
| "x_api_key", | ||
| } | ||
| ) | ||
| _SENSITIVE_ASSIGNMENT = re.compile( | ||
| r"(?i)(?<![a-z0-9_-])(?:access[-_]?token|auth[-_]?token|refresh[-_]?token|" | ||
| r"x[-_]?api[-_]?key|api[-_]?key|apikey|authorization|client[-_]?secret|" | ||
| r"credential|github[-_]?token|password|private[-_]?key|secret|token)" | ||
| r"\s*[:=]\s*[^\s,;}&\]]+" | ||
| ) | ||
| _URL_WITH_USERINFO = re.compile(r"https?://[^/@\s]+@", re.IGNORECASE) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a valid URL whose query starts immediately after the host, such as AGENTS.md reference: AGENTS.md:L65-L70 Useful? React with 👍 / 👎. |
||
| _SENSITIVE_VALUE_PATTERNS = ( | ||
| re.compile(r"\bAKIA[0-9A-Z]{16}\b"), | ||
| re.compile(r"\bgh[pousr]_[A-Za-z0-9]{36,}\b"), | ||
| re.compile(r"\bgithub_pat_[A-Za-z0-9_]{20,}\b"), | ||
| re.compile(r"\bxox[bpors]-[A-Za-z0-9-]{10,}\b"), | ||
| re.compile(r"-----BEGIN (?:RSA |EC |DSA )?PRIVATE KEY-----"), | ||
| re.compile(r"\bsecret_[A-Za-z0-9]{20,}\b"), | ||
| re.compile(r"\bntn_[A-Za-z0-9]{20,}\b"), | ||
| re.compile(r"-----BEGIN (?:(?:RSA|EC|DSA|OPENSSH|ENCRYPTED) )?PRIVATE KEY-----"), | ||
| ) | ||
|
|
||
|
|
||
| def _normalized_field_name(value: object) -> str: | ||
| return re.sub(r"[^a-z0-9]+", "_", str(value).casefold()).strip("_") | ||
|
|
||
|
|
||
| def contains_sensitive_data(value: Any) -> bool: | ||
| """Return whether JSON-compatible data contains credential data.""" | ||
| if isinstance(value, dict): | ||
| return any( | ||
| str(key).lower() in _SENSITIVE_FIELD_NAMES or contains_sensitive_data(item) | ||
| _normalized_field_name(key) in _SENSITIVE_FIELD_NAMES | ||
| or contains_sensitive_data(item) | ||
| for key, item in value.items() | ||
| ) | ||
| if isinstance(value, list): | ||
| return any(contains_sensitive_data(item) for item in value) | ||
| if isinstance(value, tuple): | ||
| return any(contains_sensitive_data(item) for item in value) | ||
| if isinstance(value, str): | ||
| return any(pattern.search(value) for pattern in _SENSITIVE_VALUE_PATTERNS) | ||
| return ( | ||
| any(pattern.search(value) for pattern in _SENSITIVE_VALUE_PATTERNS) | ||
| or _SENSITIVE_ASSIGNMENT.search(value) is not None | ||
| or _URL_WITH_USERINFO.search(value) is not None | ||
| ) | ||
| return False | ||
|
|
||
|
|
||
| def _url_has_sensitive_query(url: str) -> bool: | ||
| return any(name.lower() in _SENSITIVE_FIELD_NAMES for name, _value in parse_qsl(urlparse(url).query)) | ||
| def _url_has_sensitive_components(url: str) -> bool: | ||
| parsed = urlparse(url) | ||
| names = ( | ||
| name | ||
| for component in (parsed.query, parsed.fragment) | ||
| for name, _value in parse_qsl(component) | ||
| ) | ||
| return any(_normalized_field_name(name) in _SENSITIVE_FIELD_NAMES for name in names) | ||
|
|
||
|
|
||
| def _url_has_embedded_credentials(url: str) -> bool: | ||
| parsed = urlparse(url) | ||
| return parsed.username is not None or parsed.password is not None | ||
|
|
||
|
|
||
| class ResponseCache: | ||
|
|
@@ -95,7 +127,13 @@ def put( | |
| response: object, | ||
| ) -> None: | ||
| """Store response data with current timestamp.""" | ||
| if _url_has_sensitive_query(url) or contains_sensitive_data(params) or contains_sensitive_data(response): | ||
| if ( | ||
| _url_has_sensitive_components(url) | ||
| or _url_has_embedded_credentials(url) | ||
| or contains_sensitive_data(url) | ||
| or contains_sensitive_data(params) | ||
| or contains_sensitive_data(response) | ||
| ): | ||
| return | ||
| path = self._path(url, params) | ||
| entry = { | ||
|
|
||
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.
In the control-center artifact path, ordinary operator text such as
OAuth refresh token: expiredorEstimate token: 5now satisfies this expression. Becausewrite_control_center_artifactsapplies the detector to the final rendered Markdown and raises before any weekly or control-center write, a non-secret queue title, summary, or recommendation containing that wording suppresses all artifacts; constrain this check to serialized credential fields or credential-shaped values and cover benign prose.AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.