T09 · Insecure Skill Coding Practices
Warning
- Location
- verify_skill.py:11
- Finding
- Test workflow writes an unencrypted private key and may leave it on disk after failure<![CDATA[ ## Vulnerability Details **File Location**: `verify_skill.py`, lines 11-24, 38-41, and 54-64 **Vulnerability Type**: Plaintext sensitive data and incomplete cleanup **Risk Level**: Medium ### Vulnerable Code ```python private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) private_pem = private_key.private_bytes( encoding=serialization.Encoding.PEM, format=serialization.PrivateFormat.TraditionalOpenSSL, encryption_algorithm=serialization.NoEncryption() ) public_pem = private_key.public_key().public_bytes( encoding=serialization.Encoding.PEM, format=serialization.PublicFormat.SubjectPublicKeyInfo ) with open("test_private.pem", "wb") as f: f.write(private_pem) with open("test_public.pem", "wb") as f: f.write(public_pem) ``` The signing subprocess failure path returns before reaching the later cleanup block: ```python if result.returncode != 0: print(f"Error: {result.stderr}") return False ``` Cleanup is scoped only to the signature-verification block: ```python try: public_key = serialization.load_pem_public_key(public_pem) public_key.verify( signature, artifact_content, padding.PKCS1v15(), hashes.SHA256() ) print("VERIFICATION SUCCESSFUL: Signature is valid!") return True except Exception as e: print(f"VERIFICATION FAILED: {e}") return False finally: # Cleanup for f in ["test_private.pem", "test_public.pem", "test_artifact.txt", "test_artifact.txt.sig"]: if os.path.exists(f): os.remove(f) ``` ### Technical Analysis The test serializes private-key material using `NoEncryption()` and writes it into the current working directory through a normal `open()` operation. The resulting permissions depend on the process umask and surrounding filesystem configuration; the code does not explicitly ensure owner-only access. The cleanup `finally` block begins only after the signing subprocess succeeds and the generated signature has b ...[truncated 1640 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Perform the entire test lifecycle inside an outer `try/finally` block so cleanup runs for subprocess failures, missing signature files, keyboard interrupts, and verification errors. - Use `tempfile.TemporaryDirectory()` to isolate all generated files in a uniquely named private temporary directory. - Create private-key files with explicit owner-only permissions such as `0o600`; on POSIX systems, use `os.open()` with `O_CREAT | O_EXCL` and the desired mode to avoid a permission window. - Avoid writing the private key when possible. Refactor the signing implementation to accept an in-memory key for tests. - If a key must persist beyond a tightly controlled test, serialize it with `BestAvailableEncryption()` and obtain the passphrase from an appropriate secret source. - Resolve `sign_artifact.py` relative to `__file__` rather than the caller's working directory, reducing avoidable test failures. - Ensure test failures produce a nonzero process exit status after cleanup. A safer structure is: ```python import os import tempfile def test_signing_flow(): with tempfile.TemporaryDirectory() as temp_dir: key_path = os.path.join(temp_dir, "test_private.pem") artifact_path = os.path.join(temp_dir, "test_artifact.txt") fd = os.open(key_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) with os.fdopen(fd, "wb") as key_file: key_file.write(private_pem) # Run signing and verification while all test files remain isolated. # TemporaryDirectory removes the files when the block exits. ``` ]]>