Enhanced VulnerableApp-facade to support Card generation and toggle b…#111
Conversation
…utton between scanner as well as challenge
|
i retest in local environment with some change . it still didnt pass header test because of new ui change ,where drop down menu is inside toggle button |
|
Hi maintainers, Thanks! |
|
Hi maintainers, just a gentle follow-up on this PR. |
| export const ChallengeCard: React.FC<ChallengeCardProps> = ({ | ||
| challenge, | ||
| challengeNumber, | ||
| }) => { |
There was a problem hiding this comment.
Payload is currently revealed automatically once all hints are shown. From a UX perspective, it would be better to gate this behind an explicit user action.
I think we should add a “Reveal Payload” button so the user intentionally chooses to view the final answer. This preserves the challenge experience and improves engagement.
There was a problem hiding this comment.
hey i want to know which payload you taking about like in challenge mode or scanner mode , where payload is with hints
There was a problem hiding this comment.
in challenge mode. example:
First hint click
Second hint click
After second hint click, user should be able to see show payload button and after clicking that button, then only payload should be shown
|
|
||
| {hints.length > 0 && ( | ||
| <div | ||
| style={{ |
There was a problem hiding this comment.
There is extensive use of inline styles within the component, which makes it harder to maintain and reuse. Can you please move styles into CSS classes instead of inline style props
preetkaran20
left a comment
preetkaran20
left a comment
There was a problem hiding this comment.
Overall Looks good to me. Thanks for the PR. Added a few comments.
Thanks,
Karan
bandicam.2026-05-10.12-17-54-219.mp4hope these changes is fine and i also brought show payload button , removed inline css and converted the fixed pixel values to rem units |
Yes this looks good to me. I think you have not pushed the changes. Also can you put the same icon 🧩? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a Challenge Mode: new challenge types and global flag, a ChallengeCard component with hint/payload reveal, Content and Header UI wiring for challenge vs scanner modes, mock challenge data, styles, tests, and small config updates. ChangesChallenge Mode Feature
Sequence DiagramsequenceDiagram
participant User
participant Header
participant App as GlobalState
participant Content
participant ChallengeCard
User->>Header: Click "Challenge Mode" button
Header->>Header: Check isChallengeAvailable()
alt Challenge Mode Available
Header->>App: setGlobalState(isChallengeModeEnabled: true)
App-->>Content: State update
Content->>Content: Render challenge cards
Content->>ChallengeCard: Pass challenge data
ChallengeCard-->>User: Display challenge text and hints
else Challenge Mode Unavailable
Header-->>User: Challenge button disabled
User->>Header: Click "Scanner Mode" button
Header->>App: setGlobalState(isChallengeModeEnabled: false)
App-->>Content: State update
Content->>Content: Render vulnerability description
Content-->>User: Display scanner UI
end
User->>ChallengeCard: Click "Show Hint >"
ChallengeCard->>ChallengeCard: Increment revealedHints
ChallengeCard-->>User: Display next hint
User->>ChallengeCard: Click "Reveal Payload"
ChallengeCard->>ChallengeCard: Toggle isPayloadRevealed
ChallengeCard-->>User: Show exploit payload
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
facade-app/src/Components/Content.tsx (1)
54-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against stale async responses when switching levels quickly.
The callback uses mutable
this.selectedLevel; out-of-order responses can inject outdated HTML/resources into the current view.🔄 Suggested fix with request token check
export class Content extends React.Component<Props, ContentState> { selectedLevel?: LevelInformation; + private contentRequestToken = 0; ... if (selectedLevel) { this.selectedLevel = selectedLevel; + const token = ++this.contentRequestToken; getResource( selectedLevel.resourceInformation.htmlResource.uri, - this._setLocalState.bind(this), + (resp) => this._setLocalState(resp, token, selectedLevel), false ); } - _setLocalState( - vulnerabilityDefinitionResponse: VulnerabilityDefinitionResponse - ) { + _setLocalState( + vulnerabilityDefinitionResponse: VulnerabilityDefinitionResponse, + token: number, + levelForResponse: LevelInformation + ) { + if (token !== this.contentRequestToken) return; if (vulnerabilityDefinitionResponse.data) { manipulateDOM("__content__", vulnerabilityDefinitionResponse.data); - if (this.selectedLevel) { - appendStaticResourcesToDocument(this.selectedLevel); - } + appendStaticResourcesToDocument(levelForResponse); } }Also applies to: 65-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@facade-app/src/Components/Content.tsx` around lines 54 - 57, The async getResource callbacks can apply stale data because they reference mutable this.selectedLevel; capture a request-specific token or the selected level id/uri before calling getResource (e.g., const reqId = this._currentRequestId++ or const requestedUri = selectedLevel.resourceInformation.htmlResource.uri) and then, inside the callback that calls this._setLocalState, check that the token or uri still matches the current this.selectedLevel (or this._currentRequestId) and abort if it does not; apply the same request-token/uri-compare pattern for both getResource usages so only the latest response updates _setLocalState.
🧹 Nitpick comments (1)
facade-app/src/interface/State.tsx (1)
65-67: ⚡ Quick winDocument the relationship between
challengeCardsandchallenge.The presence of both
challengeCards?: Array<Challenge>andchallenge?: Challengewithout documentation creates ambiguity. Add JSDoc comments explaining:
- When each field should be used
- Whether they're mutually exclusive
- The fallback behavior (as implemented in Content.tsx)
This will help future maintainers understand the design intent.
📚 Suggested documentation
resourceInformation: ResourceInformation; - // Challenge mode + /** + * Challenge mode data. If both fields are present, challengeCards takes precedence. + * - challengeCards: Array of multiple challenge scenarios for this level + * - challenge: Single challenge (legacy format or simple use case) + */ challengeCards?: Array<Challenge>; challenge?: Challenge; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@facade-app/src/interface/State.tsx` around lines 65 - 67, Add JSDoc comments above the State interface properties challengeCards and challenge describing when to use each: explain that challengeCards?: Array<Challenge> is used for "challenge mode" multi-card displays, challenge?: Challenge is used when a single active challenge is selected, whether they are mutually exclusive (e.g., only one should be populated at a time) and which one is the fallback (state should use challenge if challengeCards is empty or undefined, matching the fallback logic in Content.tsx). Reference the Content.tsx behavior in the comment so maintainers can trace the implemented fallback and any precedence rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@facade-app/public/mockResponse.json`:
- Line 6: The "description" field in facade-app/public/mockResponse.json
currently contains placeholder/gibberish; replace its value with a concise,
user-facing sentence that accurately describes the item or feature represented
by this mock (update the "description" JSON property), e.g., a short meaningful
copy that fits the UI context and snapshot tests; ensure the new text uses
proper spelling and punctuation and remains within expected length constraints
for the UI component.
- Line 148: The JSON "description" string contains HTML anchor tags with
target="_blank" which expose window.opener; update each anchor in the
"description" field (the string in mockResponse.json) to include rel="noopener
noreferrer" alongside target="_blank" (e.g., change <a href="..."
target="_blank"> to <a href="..." target="_blank" rel="noopener noreferrer">) so
all external links are safe when rendered.
In `@facade-app/src/Components/ChallengeCard.tsx`:
- Around line 74-76: The hint HTML is rendered directly via
dangerouslySetInnerHTML with hint.text in the ChallengeCard component; sanitize
it first using DOMPurify to prevent XSS. Add DOMPurify as a dependency, import
it in ChallengeCard.tsx, run DOMPurify.sanitize(hint.text) and pass the
sanitized string to dangerouslySetInnerHTML instead of raw hint.text; apply the
same sanitization to the similar dangerouslySetInnerHTML usages in Content.tsx
to cover both instances. Ensure imports/reference to DOMPurify and variable
names (hint, hint.text, dangerouslySetInnerHTML) are updated accordingly.
In `@facade-app/src/Components/Header.tsx`:
- Around line 198-207: The wrap of RSuite interactive items with anchor tags
breaks keyboard navigation; remove the surrounding <a> elements and instead use
RSuite components' routing props: for RSuiteNav.Item set its href prop directly
(e.g., RSuiteNav.Item href="/scanner/..."), and for RSuiteDropDown.Item use the
as prop with your router Link component (e.g., RSuiteDropDown.Item as={Link}
href="/scanner/...")—import the appropriate Link from Next.js or React Router
and apply the same pattern to the other instances of Nav.Item and Dropdown.Item
mentioned in the review.
In `@facade-app/src/interface/State.tsx`:
- Around line 14-20: Remove the temporary development comment above the
ChallengeHint interface in State.tsx (the line containing "NEW - matches your
JSON") and either delete it or replace it with a concise JSDoc-style comment
that describes the purpose and usage of the ChallengeHint interface (fields
order: number, text: string) so the ChallengeHint export has proper
documentation rather than a dev note.
- Around line 44-51: Remove the development artifacts from the Challenge
interface comment and inline field note: delete the "(UPDATED)" text in the
JSDoc header above the Challenge interface and remove or replace the "✅ updated"
inline comment on the hints field; if you need to preserve context, convert them
into a concise JSDoc description for the interface or the hints property (e.g.,
document that hints is optional and an array of ChallengeHint) so the symbols
Challenge, challengeText, hints, and payload remain properly documented without
emoji/dev notes.
---
Outside diff comments:
In `@facade-app/src/Components/Content.tsx`:
- Around line 54-57: The async getResource callbacks can apply stale data
because they reference mutable this.selectedLevel; capture a request-specific
token or the selected level id/uri before calling getResource (e.g., const reqId
= this._currentRequestId++ or const requestedUri =
selectedLevel.resourceInformation.htmlResource.uri) and then, inside the
callback that calls this._setLocalState, check that the token or uri still
matches the current this.selectedLevel (or this._currentRequestId) and abort if
it does not; apply the same request-token/uri-compare pattern for both
getResource usages so only the latest response updates _setLocalState.
---
Nitpick comments:
In `@facade-app/src/interface/State.tsx`:
- Around line 65-67: Add JSDoc comments above the State interface properties
challengeCards and challenge describing when to use each: explain that
challengeCards?: Array<Challenge> is used for "challenge mode" multi-card
displays, challenge?: Challenge is used when a single active challenge is
selected, whether they are mutually exclusive (e.g., only one should be
populated at a time) and which one is the fallback (state should use challenge
if challengeCards is empty or undefined, matching the fallback logic in
Content.tsx). Reference the Content.tsx behavior in the comment so maintainers
can trace the implemented fallback and any precedence rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d2a44ee-320c-41dc-8697-70f96839b2c9
⛔ Files ignored due to path filters (7)
facade-app/package-lock.jsonis excluded by!**/package-lock.jsonfacade-app/src/test/__snapshots__/App.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/CommandInjection.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/Footer.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/JwtVulnerability.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/UnrestrictedFileUpload.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
facade-app/public/mockResponse.jsonfacade-app/src/App.tsxfacade-app/src/Components/ChallengeCard.tsxfacade-app/src/Components/Content.tsxfacade-app/src/Components/Header.tsxfacade-app/src/interface/State.tsxfacade-app/src/react-app-env.d.tsfacade-app/src/styles/ChallengeCard.cssfacade-app/src/test/Header.test.tsxfacade-app/tsconfig.json
| { | ||
| "name": "AuthenticationVulnerability", | ||
| "id": "AuthenticationVulnerability", | ||
| "description": "hi bro ia ma sfasdfasdffal;sdkf;klasdfjldsfkdfjdsalkfjdfjdfldsfjdlflajdsfkdfjkd", |
There was a problem hiding this comment.
Replace placeholder/gibberish description with meaningful copy.
This value appears user-facing and currently reads as test noise, which will hurt UI quality and snapshots.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@facade-app/public/mockResponse.json` at line 6, The "description" field in
facade-app/public/mockResponse.json currently contains placeholder/gibberish;
replace its value with a concise, user-facing sentence that accurately describes
the item or feature represented by this mock (update the "description" JSON
property), e.g., a short meaningful copy that fits the UI context and snapshot
tests; ensure the new text uses proper spelling and punctuation and remains
within expected length constraints for the UI component.
| key={hIndex} | ||
| dangerouslySetInnerHTML={{ __html: hint.text }} | ||
| className="hint-list-item" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ChallengeCard.tsx" -type fRepository: SasanLabs/VulnerableApp-facade
Length of output: 120
🏁 Script executed:
rg "dangerouslySetInnerHTML" --type tsx --type ts -nRepository: SasanLabs/VulnerableApp-facade
Length of output: 102
🏁 Script executed:
rg "DOMPurify" --type tsx --type ts --type json -nRepository: SasanLabs/VulnerableApp-facade
Length of output: 102
🏁 Script executed:
cat -n ./facade-app/src/Components/ChallengeCard.tsx | head -100Repository: SasanLabs/VulnerableApp-facade
Length of output: 3794
🏁 Script executed:
rg "dangerouslySetInnerHTML" --type js -nRepository: SasanLabs/VulnerableApp-facade
Length of output: 56
🏁 Script executed:
rg "DOMPurify" -nRepository: SasanLabs/VulnerableApp-facade
Length of output: 56
🏁 Script executed:
rg "dangerouslySetInnerHTML" -nRepository: SasanLabs/VulnerableApp-facade
Length of output: 379
🏁 Script executed:
cat -n ./facade-app/src/Components/Content.tsx | grep -A 2 -B 2 "dangerouslySetInnerHTML"Repository: SasanLabs/VulnerableApp-facade
Length of output: 561
🏁 Script executed:
rg "hints" facade-app/src/Components/ChallengeCard.tsx -A 3 -B 1Repository: SasanLabs/VulnerableApp-facade
Length of output: 1493
🏁 Script executed:
rg "hintCards" -n --type jsRepository: SasanLabs/VulnerableApp-facade
Length of output: 56
🏁 Script executed:
grep -r "sanitize\|escape\|htmlspecialchars" facade-app/src --include="*.ts" --include="*.tsx" -iRepository: SasanLabs/VulnerableApp-facade
Length of output: 591
🏁 Script executed:
cat facade-app/package.json | grep -A 20 "dependencies"Repository: SasanLabs/VulnerableApp-facade
Length of output: 707
Sanitize hint HTML before rendering to prevent XSS.
hint.text is injected as raw HTML without sanitization and should be escaped. The hints data comes from challenge.hints or the fallback challenge.hintCards source.
Add DOMPurify to dependencies and sanitize before rendering:
Suggested fix
+ import DOMPurify from "dompurify";
...
- <li
- key={hIndex}
- dangerouslySetInnerHTML={{ __html: hint.text }}
- className="hint-list-item"
- />
+ <li
+ key={hIndex}
+ dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(hint.text) }}
+ className="hint-list-item"
+ />Note: Similar unsanitized dangerouslySetInnerHTML patterns exist in Content.tsx (lines 128 and 221).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key={hIndex} | |
| dangerouslySetInnerHTML={{ __html: hint.text }} | |
| className="hint-list-item" | |
| key={hIndex} | |
| dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(hint.text) }} | |
| className="hint-list-item" |
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 74-74: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@facade-app/src/Components/ChallengeCard.tsx` around lines 74 - 76, The hint
HTML is rendered directly via dangerouslySetInnerHTML with hint.text in the
ChallengeCard component; sanitize it first using DOMPurify to prevent XSS. Add
DOMPurify as a dependency, import it in ChallengeCard.tsx, run
DOMPurify.sanitize(hint.text) and pass the sanitized string to
dangerouslySetInnerHTML instead of raw hint.text; apply the same sanitization to
the similar dangerouslySetInnerHTML usages in Content.tsx to cover both
instances. Ensure imports/reference to DOMPurify and variable names (hint,
hint.text, dangerouslySetInnerHTML) are updated accordingly.
| /** | ||
| * Challenge Mode Hint (NEW - matches your JSON) | ||
| */ | ||
| export interface ChallengeHint { | ||
| order: number; | ||
| text: string; | ||
| } |
There was a problem hiding this comment.
Remove development comments before merging.
The comment "NEW - matches your JSON" appears to be a temporary development note. Replace it with proper documentation explaining the purpose and usage of ChallengeHint, or remove it entirely.
📝 Suggested documentation improvement
/**
- * Challenge Mode Hint (NEW - matches your JSON)
+ * Hint for Challenge Mode with sequential ordering.
+ * Used to progressively guide users toward vulnerability exploitation.
*/
export interface ChallengeHint {
order: number;
text: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Challenge Mode Hint (NEW - matches your JSON) | |
| */ | |
| export interface ChallengeHint { | |
| order: number; | |
| text: string; | |
| } | |
| /** | |
| * Hint for Challenge Mode with sequential ordering. | |
| * Used to progressively guide users toward vulnerability exploitation. | |
| */ | |
| export interface ChallengeHint { | |
| order: number; | |
| text: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@facade-app/src/interface/State.tsx` around lines 14 - 20, Remove the
temporary development comment above the ChallengeHint interface in State.tsx
(the line containing "NEW - matches your JSON") and either delete it or replace
it with a concise JSDoc-style comment that describes the purpose and usage of
the ChallengeHint interface (fields order: number, text: string) so the
ChallengeHint export has proper documentation rather than a dev note.
| /** | ||
| * Challenge Mode Structure (UPDATED) | ||
| */ | ||
| export interface Challenge { | ||
| challengeText: string; | ||
| hints?: Array<ChallengeHint>; // ✅ updated | ||
| payload?: Payload; | ||
| } |
There was a problem hiding this comment.
Clean up development artifacts.
Lines 45 and 49 contain development notes ("(UPDATED)" and "✅ updated") that should be removed or converted to proper documentation before merging.
📝 Suggested cleanup
/**
- * Challenge Mode Structure (UPDATED)
+ * Challenge Mode structure containing the challenge text, progressive hints,
+ * and an optional payload for the solution.
*/
export interface Challenge {
challengeText: string;
- hints?: Array<ChallengeHint>; // ✅ updated
+ hints?: Array<ChallengeHint>;
payload?: Payload;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Challenge Mode Structure (UPDATED) | |
| */ | |
| export interface Challenge { | |
| challengeText: string; | |
| hints?: Array<ChallengeHint>; // ✅ updated | |
| payload?: Payload; | |
| } | |
| /** | |
| * Challenge Mode structure containing the challenge text, progressive hints, | |
| * and an optional payload for the solution. | |
| */ | |
| export interface Challenge { | |
| challengeText: string; | |
| hints?: Array<ChallengeHint>; | |
| payload?: Payload; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@facade-app/src/interface/State.tsx` around lines 44 - 51, Remove the
development artifacts from the Challenge interface comment and inline field
note: delete the "(UPDATED)" text in the JSDoc header above the Challenge
interface and remove or replace the "✅ updated" inline comment on the hints
field; if you need to preserve context, convert them into a concise JSDoc
description for the interface or the hints property (e.g., document that hints
is optional and an array of ChallengeHint) so the symbols Challenge,
challengeText, hints, and payload remain properly documented without emoji/dev
notes.
There was a problem hiding this comment.
@preetkaran20, sure! Here's the plan to clean up those development artifacts from facade-app/src/interface/State.tsx:
- Remove
(UPDATED)from theChallengeinterface JSDoc and replace it with a meaningful description - Remove the
// ✅ updatedinline comment from thehintsfield
Failed to handle agent chat message. Please try again.
|
hey maintainer this time i checked all conditions such as- Please approve the workflow so CI checks can run. Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@facade-app/src/Components/ChallengeCard.tsx`:
- Around line 42-50: The fallback for hints uses (challenge as any).hintCards
with no validation and can throw if hintCards or hc.hints are not arrays; update
the logic around the hints assignment so you first check
Array.isArray((challenge as any).hintCards), then iterate only over items where
hc is an object and Array.isArray(hc.hints), use a safe flatMap/filter to
collect strings (coerce or skip non-strings) and map them to { order, text }
entries; reference the existing hints variable, the hintCards property, and the
flatMap usage when replacing the current unguarded fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a60111a7-b457-4eef-aa98-57bd00459383
⛔ Files ignored due to path filters (3)
facade-app/package-lock.jsonis excluded by!**/package-lock.jsonfacade-app/src/test/__snapshots__/App.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
facade-app/package.jsonfacade-app/public/mockResponse.jsonfacade-app/src/App.tsxfacade-app/src/Components/ChallengeCard.tsxfacade-app/src/Components/Header.tsxfacade-app/src/test/Header.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- facade-app/public/mockResponse.json
- facade-app/src/Components/Header.tsx
- facade-app/src/App.tsx
preetkaran20
left a comment
preetkaran20
left a comment
There was a problem hiding this comment.
Overall looks good to me. please fix small cosmetic changes and we are good to merge it.
| /** | ||
| * Challenge Mode Structure (UPDATED) | ||
| */ | ||
| export interface Challenge { | ||
| challengeText: string; | ||
| hints?: Array<ChallengeHint>; // ✅ updated | ||
| payload?: Payload; | ||
| } |
|
|
||
| <RSuiteNav pullRight> | ||
| <RSuiteNav.Item style={{ padding: "0 5px" }}> | ||
| <div |
There was a problem hiding this comment.
can we avoid this inline styling?
|
|
||
| const effectiveMode = | ||
| globalState.isChallengeModeEnabled && !isChallengeDisabled | ||
| ? "challenge" |
There was a problem hiding this comment.
Better to create an Enum for effectiveMode such that we don't have to do string comparison
|
|
||
| if (!applicationData) return false; | ||
|
|
||
| const app = applicationData.find( |
There was a problem hiding this comment.
we have an object defined for ApplicationState, can you please use that?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
===========================================
- Coverage 86.39% 70.16% -16.23%
===========================================
Files 9 10 +1
Lines 147 238 +91
Branches 38 95 +57
===========================================
+ Hits 127 167 +40
- Misses 18 69 +51
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
facade-app/src/Components/Header.tsx (1)
107-140: ⚡ Quick winAdd accessibility attributes to mode toggle buttons.
The toggle buttons lack ARIA attributes for screen readers:
- Add
aria-pressedto indicate toggle state- Add
titleattribute to explain why Challenge Mode is disabled- Consider adding
aria-labelfor clarity♿ Proposed accessibility improvements
<button disabled={isChallengeDisabled} + aria-pressed={effectiveMode === AppMode.CHALLENGE} + aria-label="Switch to Challenge Mode" + title={isChallengeDisabled ? "No challenges available for this page" : "Switch to Challenge Mode"} onClick={() => setGlobalState({ ...globalState, isChallengeModeEnabled: true, }) } className={`mode-button challenge-btn ${ effectiveMode === AppMode.CHALLENGE ? "mode-button-active" : "" } ${isChallengeDisabled ? "mode-button-disabled" : ""}`} > Challenge Mode </button> {/* Scanner Mode */} <button + aria-pressed={effectiveMode === AppMode.SCANNER} + aria-label="Switch to Scanner Mode" onClick={() => setGlobalState({ ...globalState, isChallengeModeEnabled: false, }) } className={`mode-button scanner-btn ${ effectiveMode === AppMode.SCANNER ? "mode-button-active" : "" }`} > Scanner Mode </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@facade-app/src/Components/Header.tsx` around lines 107 - 140, The two mode toggle buttons (the Challenge Mode button using isChallengeDisabled and the Scanner Mode button using setGlobalState/globalState and isChallengeModeEnabled) need ARIA attributes: add aria-pressed to each button (true when effectiveMode === AppMode.CHALLENGE for the Challenge button, and true when effectiveMode === AppMode.SCANNER for the Scanner button), add an aria-label on each button that clearly describes the action (e.g., "Enable Challenge Mode" / "Enable Scanner Mode"), and add a title on the Challenge Mode button that conditionally shows the reason it is disabled (derive the message when isChallengeDisabled is true). Ensure these attributes are placed on the same button elements that reference isChallengeDisabled, effectiveMode, and setGlobalState.facade-app/src/styles/Header.css (1)
6-9: ⚡ Quick winConsider reducing z-index: 9999 to a more standard value.
While no actual z-index conflicts exist in the codebase, a value of 9999 is excessive for a navbar element. RSuite components typically use z-index values around 1050 by default. Reducing this to 1000-1050 aligns with best practices and keeps the header above standard UI elements without being unnecessarily high.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@facade-app/src/styles/Header.css` around lines 6 - 9, The .header-container currently uses an excessive z-index of 9999; change the value to a more standard level (e.g., 1000–1050) in the Header.css rule for .header-container so the navbar stays above typical UI layers without using an overly large z-index; update the z-index declaration in the .header-container selector accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@facade-app/src/Components/ChallengeCard.tsx`:
- Around line 42-56: The code uses (challenge as any).hintCards; add a proper
optional type for hintCards (e.g., hintCards?: Array<{ hints: string[] }>) to
the existing Challenge interface or to ChallengeCardProps so the component sees
hintCards typed, update the ChallengeCard component prop type to use that
interface, then remove all (challenge as any) casts and access
challenge.hintCards directly in the block that builds hints (filter, flatMap,
map) so TypeScript knows hintCards is Array<{ hints: string[] }>.
---
Nitpick comments:
In `@facade-app/src/Components/Header.tsx`:
- Around line 107-140: The two mode toggle buttons (the Challenge Mode button
using isChallengeDisabled and the Scanner Mode button using
setGlobalState/globalState and isChallengeModeEnabled) need ARIA attributes: add
aria-pressed to each button (true when effectiveMode === AppMode.CHALLENGE for
the Challenge button, and true when effectiveMode === AppMode.SCANNER for the
Scanner button), add an aria-label on each button that clearly describes the
action (e.g., "Enable Challenge Mode" / "Enable Scanner Mode"), and add a title
on the Challenge Mode button that conditionally shows the reason it is disabled
(derive the message when isChallengeDisabled is true). Ensure these attributes
are placed on the same button elements that reference isChallengeDisabled,
effectiveMode, and setGlobalState.
In `@facade-app/src/styles/Header.css`:
- Around line 6-9: The .header-container currently uses an excessive z-index of
9999; change the value to a more standard level (e.g., 1000–1050) in the
Header.css rule for .header-container so the navbar stays above typical UI
layers without using an overly large z-index; update the z-index declaration in
the .header-container selector accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0ff9d54-04d3-4b20-a019-ff054b97e930
⛔ Files ignored due to path filters (2)
facade-app/src/test/__snapshots__/App.test.tsx.snapis excluded by!**/*.snapfacade-app/src/test/__snapshots__/Header.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
facade-app/src/Components/ChallengeCard.tsxfacade-app/src/Components/Header.tsxfacade-app/src/interface/State.tsxfacade-app/src/styles/Header.css
🚧 Files skipped from review as they are similar to previous changes (1)
- facade-app/src/interface/State.tsx
|
hey maintainer i implemented all requested changes . please review it |
preetkaran20
left a comment
preetkaran20
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot @saurabhkushwaha438 for such a great PR.
|
🎉 Thanks for contributing @saurabhkushwaha438! We’d love to stay in touch and grow SasanLabs 🚀 Also consider ⭐ starring the repo if you like it! |
2 participants
✨ Enhancing VulnerableApp-Facade: Card Generation & Mode Toggle (Scanner ↔ Challenge)
📌 Overview
This PR enhances the VulnerableApp-Facade by introducing:
These changes improve usability and make it easier for users to interact with different application flows.
🚀 Key Changes
🔹 Card Generation
🔹 Mode Toggle (Scanner ↔ Challenge)
Added a toggle button to switch between:
Ensures smooth transition without breaking existing functionality
Maintains state consistency across views
🧪 Testing
✅ Verified functionality using local Docker setup
✅ Tested both modes:
✅ Confirmed UI updates reflect correctly in:
http://localhost(Docker)http://localhost:3000(dev server)✅ Ensured no regression in existing features
⚙️ Additional Notes
🙌 Summary
This PR improves the usability and flexibility of the VulnerableApp-Facade by enabling dynamic UI rendering and seamless mode switching, aligning with the goals of issue #570.
Closes #570
Summary by CodeRabbit
New Features
Style
Chores
Tests