shop.py 코드 리뷰

• 대상: shop.py (84줄, sqlite3 기반 주문 처리 모듈)
• 검토일: 2026-10-16
• 결론: ❌ 이대로 머지하면 안 됩니다. 실제 서비스에 올리기 전에 아래 🔴 Critical 6건은 반드시 고쳐야 합니다.

등급	개수	요약
🔴 Critical	6	시크릿 노출, SQL 인젝션, 남의 주문 조회(IDOR), 경로 탐색, 음수 수량으로 돈·재고 생성, 동시 주문 시 초과 판매
🟠 High	4	잔액 검증 없음, 없는 상품 주문 시 크래시, 쿠폰 오류를 조용히 무시, 페이지네이션 버그
🟡 Medium	4	N+1 쿼리, DB 연결 누수, 상대 경로 의존, 테스트 없음

---

🔴 Critical — 머지 전 필수 수정

1. 운영 관리자 토큰이 코드에 하드코딩됨 (7행, 83–84행)

ADMIN_TOKEN = "sk_live_9f8e7d6c5b4a3210shopadmin"

• sk_live_ 접두사로 보아 실제 운영용 키로 보입니다. 저장소에 접근할 수 있는 사람이라면 누구나 관리자 권한을 얻습니다.
• 이미 공유된 코드이므로 키를 즉시 폐기하고 재발급해야 합니다. 코드만 고치고 키를 그대로 두면 소용이 없습니다. git 히스토리에 남았다면 히스토리 정리도 필요합니다.
• token == ADMIN_TOKEN은 타이밍 공격에 취약합니다.

수정

import hmac
ADMIN_TOKEN = os.environ["SHOP_ADMIN_TOKEN"]

def is_admin(token):
    return isinstance(token, str) and hmac.compare_digest(token, ADMIN_TOKEN)

2. SQL 인젝션 (19행 search_products)

sql = f"SELECT ... WHERE name LIKE '%{keyword}%'"

• 사용자 검색어가 SQL 문자열에 그대로 들어갑니다. 예를 들어 keyword = "' UNION SELECT id, email, password FROM users --"를 넣으면 회원 테이블이 통째로 유출됩니다.

수정

db.execute("SELECT id, name, price FROM products WHERE name LIKE ?", (f"%{keyword}%",))

(%, _ 같은 와일드카드까지 막으려면 ESCAPE '\'도 함께 쓰세요.)

3. 다른 사람의 주문을 볼 수 있음, IDOR (40–43행 get_order)

• current_user_id 인자를 받지만 쓰지 않습니다. 주문 ID를 1, 2, 3… 순서대로 바꿔 넣으면 모든 고객의 주문 정보를 볼 수 있습니다.
• SELECT *라서 앞으로 컬럼이 추가되면 (주소, 연락처 등) 그것까지 노출됩니다.

수정

row = db.execute(
    "SELECT id, user_id, created_at FROM orders WHERE id = ? AND user_id = ?",
    (order_id, current_user_id),
).fetchone()

4. 경로 탐색으로 서버 파일 읽기 가능 (77–80행 download_invoice)

• filename = "../../etc/passwd"나 "../shop.db"를 넣으면 DB 파일을 포함해 서버의 아무 파일이나 내려받을 수 있습니다. os.path.join은 절대 경로("/etc/passwd")가 들어오면 앞부분을 아예 버립니다.
• 소유자 확인도 없어서, 경로 문제를 막더라도 남의 송장을 받을 수 있습니다.

수정: 파일명 대신 order_id를 받아 소유권을 확인한 뒤 서버가 경로를 만들도록 바꾸세요. 파일명을 꼭 받아야 한다면 경로를 정규화한 후 디렉터리 안인지 확인해야 합니다.

base = os.path.realpath(INVOICE_DIR)
path = os.path.realpath(os.path.join(base, filename))
if os.path.commonpath([base, path]) != base:
    raise PermissionError("invalid path")

5. 음수 수량으로 재고와 잔액을 늘릴 수 있음 (46–66행 place_order)

• qty를 검증하지 않습니다. qty = -100이면:
• 재고 확인(stock < -100)을 그냥 통과하고
• stock - (-100) → 재고가 늘어나고
• balance - price * (-100) → 사용자 잔액이 늘어납니다. 사실상 돈을 마음대로 찍어낼 수 있습니다.
• qty = 0이어도 빈 주문이 생성됩니다.

수정: 함수 맨 앞에 if not isinstance(qty, int) or qty <= 0: raise ValueError(...)를 넣으세요.

6. 동시 주문 시 초과 판매됨, 경쟁 상태 (48–55행)

• "재고 조회 → 확인 → 차감"이 각각 따로 실행됩니다. 재고가 1개 남았을 때 두 요청이 동시에 들어오면 둘 다 stock = 1을 읽고 통과해서 재고가 -1이 됩니다. 한정 수량 이벤트에서 바로 문제가 됩니다.

수정: 조건부 UPDATE로 확인과 차감을 한 번에 처리하고, 쓰기 트랜잭션을 처음부터 잡으세요.

db.execute("BEGIN IMMEDIATE")
cur = db.execute(
    "UPDATE products SET stock = stock - ? WHERE id = ? AND stock >= ?",
    (qty, product_id, qty),
)
if cur.rowcount != 1:
    db.rollback()
    raise ValueError("재고 부족")

---

🟠 High

7. 잔액 검증이 없음 (61–64행)

• 잔액이 부족해도 결제가 진행되어 잔액이 음수가 됩니다. 재고와 같은 방식으로 ... WHERE id = ? AND balance >= ? 처리 후 rowcount를 확인하고, 실패하면 전체를 롤백하세요.

8. 없는 상품을 주문하면 크래시 (51행)

• 상품이 없으면 stock이 None이라서 stock["stock"]에서 TypeError가 납니다. 사용자에게 500 에러가 보입니다. None이면 명확한 예외를 던지도록 처리하세요.

9. apply_coupon이 오류를 조용히 삼킴 (69–74행)

• except Exception: pass 때문에 쿠폰 형식이 잘못되면 None을 반환합니다. 호출하는 쪽에서 결제 금액이 None이 되어 엉뚱한 곳에서 터지거나, 최악의 경우 0원으로 처리될 수 있습니다.
• percent 범위 검사가 없습니다. percent = 150이면 결제 금액이 음수가 되고, 음수 퍼센트면 오히려 가격이 오릅니다.
• 금액을 float로 계산해서 반올림 오차가 생깁니다. 원 단위 정수나 Decimal을 쓰세요.

수정

def apply_coupon(total, coupon):
    percent = coupon["percent"]  # 형식 오류는 그대로 예외로 올림
    if not 0 <= percent <= 100:
        raise ValueError("invalid coupon")
    return total - (total * percent // 100)

10. 페이지네이션 버그 (25행, 27행 list_orders)

• offset = page * PAGE_SIZE이고 기본값이 page=1이라서, 첫 페이지 요청 시 최근 주문 20건을 건너뜁니다. (page - 1) * PAGE_SIZE로 고쳐야 합니다.
• ORDER BY가 없어서 페이지마다 순서가 보장되지 않습니다. 같은 주문이 중복으로 나오거나 누락될 수 있습니다. ORDER BY created_at DESC, id DESC를 추가하세요.
• created_at을 조회하지만 결과에 담지 않습니다.
• page가 0 이하인 경우를 검증하지 않습니다.

---

🟡 Medium

11. N+1 쿼리 (31–36행)

• 주문 20건이면 쿼리가 21번 실행됩니다. WHERE order_id IN (...) 한 번이나 JOIN으로 묶으세요.

12. DB 연결을 닫지 않음 (모든 함수)

• get_db()로 연 연결을 닫지 않습니다. 트래픽이 많으면 파일 핸들이 누수되고, 예외가 났을 때 롤백 시점이 GC에 달려 있어서 락이 오래 유지될 수 있습니다. contextlib.closing이나 with 블록으로 관리하세요. (sqlite3의 with conn:은 커밋/롤백만 처리하고 연결을 닫지는 않습니다.)

13. 상대 경로에 의존 (5–6행)

• "shop.db", "invoices"는 프로세스의 작업 디렉터리에 따라 위치가 달라집니다. 실행 위치가 바뀌면 빈 DB가 새로 생성될 수 있습니다. 환경 변수나 절대 경로를 쓰세요.

14. 테스트가 없음

• 돈과 재고를 다루는 코드인데 테스트가 없습니다. 최소한 다음 경우는 테스트로 고정해 두길 권합니다.
• 음수·0 수량
• 재고 부족, 잔액 부족
• 없는 상품
• 동시 주문 2건
• 남의 주문 조회
• ../ 파일명
• 쿠폰 0·100·150%
• 첫 페이지 결과

---

머지 전 체크리스트

• [ ] ADMIN_TOKEN 폐기 및 재발급, 환경 변수로 이동, hmac.compare_digest 사용
• [ ] search_products 파라미터 바인딩
• [ ] get_order 소유자 조건 추가
• [ ] download_invoice 경로 검증과 소유권 확인
• [ ] place_order: 수량 검증, 상품 존재 확인, 조건부 UPDATE로 재고·잔액 원자적 차감, 실패 시 롤백
• [ ] apply_coupon 예외 삼킴 제거, 범위 검증, 정수/Decimal 금액
• [ ] list_orders offset 수정과 ORDER BY 추가
• [ ] 위 시나리오 테스트 작성

참고로 이 모듈에는 인증 로직이 없어서 user_id를 호출하는 쪽이 신뢰할 수 있는 세션에서 가져온다는 전제를 깔고 있습니다. API 레이어에서 클라이언트가 보낸 user_id를 그대로 넘기고 있다면 별도의 Critical 이슈가 되니 함께 확인해 보세요.