Feat/130 fr int 06 generate follow up questions#138
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 AI 면접 시스템의 핵심 기능을 확장하고 보안을 강화하는 데 중점을 둡니다. 지원자의 이력서와 채용 공고 정보를 AI 프롬프트에 동적으로 주입하여 더욱 정교하고 개인화된 꼬리 질문을 생성할 수 있게 되었으며, 면접 완료 후 결과를 확인할 수 있는 리포트 조회 API를 도입했습니다. 또한, 사용자 세션에 대한 IDOR 취약점을 방어하고 프롬프트 파일 로드 시 Path Traversal 공격을 방지하는 등 전반적인 시스템 보안 수준을 향상했습니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR implements AI-customized follow-up question generation and an interview result report inquiry API. While it introduces important functionality and includes some security improvements like Path Traversal protection, a significant IDOR vulnerability exists due to the current method of verifying session ownership via a user-supplied memberId parameter. Furthermore, the AI prompt construction is vulnerable to prompt injection. For the report inquiry API, consider using more appropriate HTTP status codes. To enhance maintainability, hardcoded strings in the Gemini prompt generation logic should be externalized. Addressing the IDOR issue with a robust authentication framework is a high priority, especially for the newly added report service.
| public ResponseEntity<InterviewReportResponse> getInterviewReport( | ||
| @PathVariable Long sessionId, | ||
| @RequestParam Long memberId) { | ||
|
|
||
| // 1. 소유권 검증: 내 면접이 맞는지 확인 (IDOR 취약점 방어) | ||
| InterviewSession session = validateAndGetSessionOwnership(sessionId, memberId); |
There was a problem hiding this comment.
The getInterviewReport endpoint (and others using validateAndGetSessionOwnership) is vulnerable to Insecure Direct Object Reference (IDOR). The memberId used for ownership verification is taken directly from the request parameters (@RequestParam Long memberId) without being verified against the actual authenticated user. An attacker can access any user's interview report by providing that user's memberId and the corresponding sessionId. As noted in the code comments, this should be replaced with a secure authentication mechanism (e.g., @AuthenticationPrincipal from Spring Security) to ensure the memberId corresponds to the logged-in user.
References
- When accessing resources via an identifier (e.g., sessionId), always verify that the resource belongs to the currently authenticated user to prevent unauthorized access.
| if (session.getResumeId() != null) { | ||
| resumeTitle = resumeRepository.findById(session.getResumeId()) | ||
| .map(Resume::getTitle) | ||
| .orElse("삭제된 이력서"); | ||
| } | ||
|
|
||
| // 채용 공고 제목 조회 | ||
| String jobTitle = null; | ||
| if (session.getJobPostingId() != null) { | ||
| jobTitle = jobPostingRepository.findById(session.getJobPostingId()) | ||
| .map(JobPosting::getJobTitle) | ||
| .orElse("삭제된 공고"); | ||
| } |
There was a problem hiding this comment.
리포트 생성 시 이력서와 채용 공고 제목을 조회할 때 findById()를 사용하고 있습니다. 이 경우, 면접 세션(session)의 소유권은 검증되었지만, 해당 세션에 연결된 이력서(resumeId)나 채용 공고(jobPostingId)가 정말 해당 사용자의 소유인지 검증하지 않아 IDOR(Insecure Direct Object Reference) 취약점이 발생할 수 있습니다.
ConversationManager에서 구현된 것처럼, memberId를 함께 사용하여 resumeRepository.findByIdAndMemberId()와 jobPostingRepository.findByIdAndMemberId()를 호출하여 소유권을 반드시 검증해야 합니다.
| if (session.getResumeId() != null) { | |
| resumeTitle = resumeRepository.findById(session.getResumeId()) | |
| .map(Resume::getTitle) | |
| .orElse("삭제된 이력서"); | |
| } | |
| // 채용 공고 제목 조회 | |
| String jobTitle = null; | |
| if (session.getJobPostingId() != null) { | |
| jobTitle = jobPostingRepository.findById(session.getJobPostingId()) | |
| .map(JobPosting::getJobTitle) | |
| .orElse("삭제된 공고"); | |
| } | |
| if (session.getResumeId() != null) { | |
| resumeTitle = resumeRepository.findByIdAndMemberId(session.getResumeId(), session.getMemberId()) | |
| .map(Resume::getTitle) | |
| .orElse("삭제된 이력서"); | |
| } | |
| // 채용 공고 제목 조회 | |
| String jobTitle = null; | |
| if (session.getJobPostingId() != null) { | |
| jobTitle = jobPostingRepository.findByIdAndMemberId(session.getJobPostingId(), session.getMemberId()) | |
| .map(JobPosting::getJobTitle) | |
| .orElse("삭제된 공고"); | |
| } |
References
- When accessing resources via an identifier (e.g., sessionId), always verify that the resource belongs to the currently authenticated user to prevent unauthorized access.
| String finalPrompt = String.format("%s%s%s\n\n%s\n\n[Candidate Answer]\n%s", | ||
| atmospherePrompt, resumeContext, jobContext, constraints, request.getMessage()); |
There was a problem hiding this comment.
The construction of finalPrompt is vulnerable to Prompt Injection. User-supplied input from request.getMessage() is directly concatenated into the prompt string. An attacker could craft a malicious response to manipulate the LLM's instructions, bypass constraints, or potentially leak the system prompt. It is recommended to use structured prompting techniques or sanitize user input to prevent the LLM from interpreting user data as instructions.
| } catch (IllegalStateException e) { | ||
| // 상태가 DONE이 아닐 경우 403 Forbidden 에러 반환 | ||
| throw new ResponseStatusException(HttpStatus.FORBIDDEN, e.getMessage()); | ||
| } |
There was a problem hiding this comment.
IllegalStateException이 발생했을 때 HttpStatus.FORBIDDEN (403)을 반환하도록 구현하셨습니다. 하지만 403은 인증된 사용자가 리소스에 접근할 권한이 없을 때 사용하는 상태 코드입니다. 이 경우, 사용자는 자신의 면접 리포트에 접근할 권한이 있지만, 면접이 아직 완료되지 않아 리소스의 상태가 요청을 처리하기에 적절하지 않은 상황입니다.
이러한 "상태 충돌" 상황에는 HttpStatus.CONFLICT (409)가 더 적합한 상태 코드입니다. API의 의미를 더 명확하게 전달하기 위해 상태 코드를 변경하는 것을 고려해 보세요.
| } catch (IllegalStateException e) { | |
| // 상태가 DONE이 아닐 경우 403 Forbidden 에러 반환 | |
| throw new ResponseStatusException(HttpStatus.FORBIDDEN, e.getMessage()); | |
| } | |
| } catch (IllegalStateException e) { | |
| // 상태가 DONE이 아닐 경우 409 Conflict 에러 반환 | |
| throw new ResponseStatusException(HttpStatus.CONFLICT, e.getMessage()); | |
| } |
| // [FR-INT-06] 이력서 기반 꼬리 질문 생성을 위한 컨텍스트 동적 주입 | ||
| String resumeContext = ""; | ||
| if (request.getResumeContent() != null && !request.getResumeContent().isBlank()) { | ||
| resumeContext = "\n\n[Candidate's Resume]\n다음은 지원자의 자기소개서 내용입니다. 이를 바탕으로 지원자의 경험을 묻는 꼬리 질문을 생성하세요.\n" + request.getResumeContent(); | ||
| } | ||
|
|
||
| // [FR-INT-07] 채용 공고 기반 맞춤형 질문 생성을 위한 컨텍스트 동적 주입 | ||
| String jobContext = ""; | ||
| if (request.getJobDescription() != null && !request.getJobDescription().isBlank()) { | ||
| jobContext = "\n\n[Job Posting Requirements]\n다음은 지원자가 지원한 채용 공고의 상세 내용(요구 역량 및 주요 업무)입니다. 이를 바탕으로 직무 적합성을 검증하는 질문을 생성하세요.\n" + request.getJobDescription(); | ||
| } |
There was a problem hiding this comment.
이력서와 채용 공고 컨텍스트를 프롬프트에 주입하기 위한 문자열이 코드에 직접 하드코딩되어 있습니다. 프롬프트는 비즈니스 로직의 중요한 부분이므로, 다른 프롬프트 파일(NORMAL.txt, constraints.txt)처럼 외부 .txt 파일로 분리하여 관리하는 것이 유지보수성 측면에서 더 좋습니다.
예를 들어, resume_context.txt와 job_context.txt 파일을 만들어 프롬프트 템플릿을 저장하고, String.format을 사용해 동적으로 내용을 채워넣는 방식을 고려해볼 수 있습니다.
2 participants
관련 이슈
작업 내용
변경 사항 및 주의 사항
체크 리스트
연관 이슈