В изменении появился экспорт задач в CSV. На входном маршруте проверяется участник проекта, поэтому первое впечатление — доступ защищён. Но файл создаёт фоновое задание, а скачивание обслуживает другой маршрут. Проверка только контроллера запуска оставит два перехода без ответа.
Разберём учебное ревью этого изменения. Его цель — проверить, может ли пользователь получить задачи чужой организации. Это не полный аудит приложения и не отчёт о найденной уязвимости в реальном проекте.
Назовите поведение, которое проверяете
Для ревью нужны исходная задача, изменение и состояние кода, на котором оно сделано. Один diff может скрывать важную зависимость: вызываемый сервис не изменился, но именно он строит выборку. Поэтому область чтения не должна ограничиваться только красными и зелёными строками.
Одновременно не надо заказывать «все проблемы безопасности». В этой проверке интересует граница организации на каждом переходе: запрос пользователя, сохранение задания, исполнение, формирование файла, выдача ссылки, скачивание. Другие классы угроз можно записать отдельными задачами.
Проверь изменение экспорта задач на доступ к чужой организации.
Прочитай diff и необходимые вызываемые методы.
Код не меняй.
Проследи запуск → задание → выборку → готовый файл → скачивание.
Для каждого перехода покажи, чем определяется организация и право доступа.
Находка: условие, путь выполнения, последствие, ссылка на код,
способ проверки. Предположение помечай отдельно.
Если проблема не подтверждается, не превращай её в находку ради отчёта.У находки должен быть сценарий
Фраза «нет проверки прав» неполна. Где её нет, кто может вызвать это место и что он получит? Возможно, метод доступен только через уже проверенную границу. Возможно, наоборот, идентификатор организации приходит из пользовательского параметра и заменяет контекст текущей сессии.
Для учебного примера кандидат на дефект можно записать так: пользователь организации А подставляет идентификатор готового экспорта Б; маршрут скачивания проверяет только вход в систему; путь к файлу берётся из записи без проверки владельца; в результате возвращаются данные Б. Это проверяемая гипотеза. Чтобы назвать её подтверждённой находкой, нужно установить, что все переходы действительно доступны и нет дополнительной проверки.
Просто существование числового идентификатора не доказывает уязвимость. Точно так же непредсказуемая ссылка сама по себе не заменяет согласованную модель доступа. Ревьюер должен исследовать фактический контракт выдачи файла, а не угадывать его по внешнему виду URL.
Покажите и проверенные, и непроверенные переходы
Короткая таблица удобна, когда у действия несколько точек входа. Она не обязана использовать только «да» и «нет»: статус «не проверено в окружении» точнее ложной уверенности.
| Переход | Что требуется установить | Возможный пробел |
|---|---|---|
| Запуск | Связь пользователя с выбранным проектом | Параметр подменяет область доступа |
| Задание | Источник идентификаторов и контекста | В воркере используется общая выборка |
| Файл | Кому принадлежит результат | Запись не сохраняет владельца |
| Скачивание | Условие выдачи конкретного результата | Проверяется только авторизация |
Эта таблица задаёт вопросы, а не утверждает, что дефекты существуют. В отчёте для каждой строки нужны найденное место, вывод и предел проверки. Доступ к хранилищу по прямому URL может зависеть от конфигурации, которой нет в репозитории.
Предложение проверки полезнее общего совета
Для подозрения на скачивание чужого файла нужен сценарий с двумя организациями и двумя пользователями. Проверяется ответ на конкретный экспорт, а не только наличие middleware в маршруте. Для фонового задания отдельно исследуется выборка: корректность входного контроллера не доказывает корректность воркера.
Если тест не запускался, так и напишите. Ревьюер может предложить минимальный сценарий без утверждения, что воспроизвёл дефект. При недоступном окружении полезная находка всё ещё возможна по коду, но степень подтверждения должна быть ясна.
Краткое направление исправления уместно, когда оно следует из причины: например, проверить принадлежность результата перед выдачей. Не требуется запрещать любые рекомендации. Важно отделить исправление обнаруженной ошибки от пожелания перестроить всю систему хранения.
Решение об исправлении остаётся у владельца изменения
После отчёта проверьте наиболее значимые выводы сами: перейдите по ссылкам, воспроизведите доступный сценарий, убедитесь, что ревьюер читал нужную версию. Агент, написавший код, может помочь с проверкой, но повторение его собственного объяснения не даёт независимого подтверждения.
Чистый результат означает только отсутствие подтверждённых замечаний в просмотренной области. Он не доказывает безопасность всей системы. Для этого экспорта ещё могут оставаться объём данных, срок хранения, формулы в CSV, ошибки повторного запуска. Перечислите их как границы текущего прохода, а не добавляйте непроверенные уязвимости в основной список.
Когда исправление готово, повторите сценарий находки и соседнее разрешённое действие: чужой файл недоступен, собственный по-прежнему скачивается. Так ревью заканчивается проверяемым изменением поведения.
