Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop sending full defences on low levels #844

Merged
merged 43 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
7bedff7
defences undefined for levels 1 and 2
pmarsh-scottlogic Feb 23, 2024
7e86ae4
fixes tests
pmarsh-scottlogic Feb 23, 2024
b0a4823
propoerly undefines defences on level 1 and 2
pmarsh-scottlogic Feb 23, 2024
4e45d90
moves variable declarations to destructureing
pmarsh-scottlogic Feb 29, 2024
d92314c
missed one
pmarsh-scottlogic Feb 29, 2024
74d8fb4
undo error swallow
pmarsh-scottlogic Feb 29, 2024
1237cca
merge dev
pmarsh-scottlogic Feb 29, 2024
dc66ffb
remove bad type guards
pmarsh-scottlogic Feb 29, 2024
c442d04
improve validation for configureDefence
pmarsh-scottlogic Feb 29, 2024
aa8f672
last bit of validation
pmarsh-scottlogic Feb 29, 2024
aa0c576
update tests for handleConfigureDefence
pmarsh-scottlogic Feb 29, 2024
d6b6455
new folder for defence controller unit tests and wrote first handleD…
pmarsh-scottlogic Feb 29, 2024
294de45
adds test for defence activation
pmarsh-scottlogic Feb 29, 2024
4785161
adds tests for defence deactivation
pmarsh-scottlogic Feb 29, 2024
66cdd2e
moves tests all back into one file again
pmarsh-scottlogic Feb 29, 2024
e74a26f
adds tests for handleResetSingleDefence
pmarsh-scottlogic Feb 29, 2024
663b22e
improve validation on handleGetDefenceStatus
pmarsh-scottlogic Feb 29, 2024
8429bfd
removes extraneous stuff from test objects
pmarsh-scottlogic Feb 29, 2024
ebba951
adds tests to make sure defences aren't chngeable for level 1
pmarsh-scottlogic Feb 29, 2024
107e453
add loop to test levels 1 and 2 with same code
pmarsh-scottlogic Feb 29, 2024
373e0b7
adds remaining tests for defence controller integration
pmarsh-scottlogic Feb 29, 2024
8293a4a
handleResetSingleDefence takes level parameter now
pmarsh-scottlogic Feb 29, 2024
ae5e746
adds level to frontend call to reset defence config item. Types all t…
pmarsh-scottlogic Feb 29, 2024
a336617
some renamings and fill in gaps in tests
pmarsh-scottlogic Feb 29, 2024
23771d9
adds another missing test
pmarsh-scottlogic Feb 29, 2024
2aab5f7
tweaks
pmarsh-scottlogic Feb 29, 2024
a04ca87
merge dev
pmarsh-scottlogic Mar 5, 2024
24b7c04
fix test suite
pmarsh-scottlogic Mar 5, 2024
2fc5a6f
replace defence search with defences.some and replace cuurrentDefence…
pmarsh-scottlogic Mar 5, 2024
e907b04
change defenceId for generic falsy check
pmarsh-scottlogic Mar 5, 2024
f56383f
improve guards for level and config
pmarsh-scottlogic Mar 5, 2024
65b5644
improve guards for level and config
pmarsh-scottlogic Mar 5, 2024
b45cce9
correct test for configuring a config item
pmarsh-scottlogic Mar 5, 2024
d670f00
updates error message when not allowed to activate or modify defences
pmarsh-scottlogic Mar 5, 2024
095ad15
rescturecure defence controller integration tests
pmarsh-scottlogic Mar 5, 2024
0d02968
change other tests to use test.each rather than foreach
pmarsh-scottlogic Mar 5, 2024
9f9d628
uses isValidLevel
pmarsh-scottlogic Mar 5, 2024
dec9904
improve validateFilterConfig
pmarsh-scottlogic Mar 5, 2024
e98c251
just pass qa defence rather than all defences
pmarsh-scottlogic Mar 7, 2024
b0e17d0
fix tests
pmarsh-scottlogic Mar 7, 2024
360c30c
uses type intersection to define QaLlmDefence
pmarsh-scottlogic Mar 12, 2024
600dc65
Merge branch 'dev' into stop-sending-full-defences-on-low-leevels
pmarsh-scottlogic Mar 12, 2024
8ec48c7
only gedDefencesFrDTOs if it is defined
pmarsh-scottlogic Mar 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions backend/src/controller/chatController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Response } from 'express';

import { defaultDefences } from '@src/defaultDefences';
import {
transformMessage,
detectTriggeredInputDefences,
Expand Down Expand Up @@ -241,14 +242,15 @@ async function handleChatToGPT(req: OpenAiChatRequest, res: Response) {
? req.session.chatModel
: defaultChatModel;

const defences =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why we want to define defences here (other than the system role) for levels 1 and 2, which leads me to wonder why handleChatWithoutDefenceDetection needs to receive the defences at all...

If you follow the passing of defences through handleChatWithoutDefenceDetection and beyond, you end up at handleAskQuestionFunction in src/openai.ts, which simply checks if the Q&A LLM defence is active. If we don't have any defences, then surely we can assume it's not active. Plus, it feels overly complex to be passing all the defences through so many calls, when all we need is the QA_LLM defence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you suggest? pass undefined through the calls instead? Make new methods for the whole call chain for getting a response without defences? I vote the former

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we don't pass defences into either handleChat function. The entire function call chain is self-contained, i.e. all functions are only invoked within this single call chain, so why not simply look up the defences from local storage in handleAskQuestionFunction? The currentLevel is passed all the way down so it's a trivial lookup. No need to pass defences down at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer is: to get something from the session, we need access to the req object

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, silly me! As discussed, let's try passing just the Q&A LLM defence down the call chain, but also have it optional so we don't need it for levels 1 and 2.

req.session.levelState[currentLevel].defences ?? defaultDefences;

const currentChatHistory = setSystemRoleInChatHistory(
currentLevel,
req.session.levelState[currentLevel].defences,
defences,
req.session.levelState[currentLevel].chatHistory
);

const defences = [...req.session.levelState[currentLevel].defences];

let levelResult: LevelHandlerResponse;
try {
if (currentLevel < LEVEL_NAMES.LEVEL_3) {
Expand Down Expand Up @@ -410,6 +412,6 @@ function handleClearChatHistory(req: OpenAiClearRequest, res: Response) {
export {
handleChatToGPT,
handleGetChatHistory,
handleAddInfoToChatHistory as handleAddInfoToChatHistory,
handleAddInfoToChatHistory,
handleClearChatHistory,
};
146 changes: 107 additions & 39 deletions backend/src/controller/defenceController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,33 +28,74 @@ function handleDefenceActivation(req: DefenceActivateRequest, res: Response) {
const defenceId = req.body.defenceId;
const level = req.body.level;
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

if (defenceId && level !== undefined) {
// activate the defence
req.session.levelState[level].defences = activateDefence(
defenceId,
req.session.levelState[level].defences
if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (level < LEVEL_NAMES.LEVEL_1 || level > LEVEL_NAMES.SANDBOX) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

if (req.session.levelState[level].defences === undefined) {
sendErrorResponse(
res,
400,
'You cannot activate defences on this level, because it uses the default defences'
);
res.status(200).send();
} else {
res.status(400).send();
return;
}

if (req.session.levelState[3].defences === undefined) return; // this dumb type guard is needed, because we are accessing the array via enum, and so typescript doesn't account for the above type guards. This will be solved with #842. The below array indices won't need to be hardcoded either.

req.session.levelState[3].defences = activateDefence(
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
defenceId,
req.session.levelState[3].defences
);
res.status(200).send();
}

function handleDefenceDeactivation(req: DefenceActivateRequest, res: Response) {
// id of the defence
const defenceId = req.body.defenceId;
const level = req.body.level;
if (defenceId && level !== undefined) {
// deactivate the defence
req.session.levelState[level].defences = deactivateDefence(
defenceId,
req.session.levelState[level].defences

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (level < LEVEL_NAMES.LEVEL_1 || level > LEVEL_NAMES.SANDBOX) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}

if (req.session.levelState[level].defences === undefined) {
sendErrorResponse(
res,
400,
'You cannot deactivate defences on this level, because it uses the default defences'
);
res.send();
} else {
res.status(400);
res.send();
return;
}

if (req.session.levelState[3].defences === undefined) return; // this dumb type guard is needed, because we are accessing the array via enum, and so typescript doesn't account for the above type guards. This will be solved with #842. The below array indices won't need to be hardcoded either.

req.session.levelState[3].defences = deactivateDefence(
defenceId,
req.session.levelState[3].defences
);
res.status(200).send();
}

function handleConfigureDefence(req: DefenceConfigureRequest, res: Response) {
Expand All @@ -73,10 +114,20 @@ function handleConfigureDefence(req: DefenceConfigureRequest, res: Response) {
return;
}

// configure the defence
req.session.levelState[level].defences = configureDefence(
if (req.session.levelState[level].defences === undefined) {
sendErrorResponse(
res,
400,
'You cannot configure defences on this level, because it uses the default defences'
);
return;
}

if (req.session.levelState[3].defences === undefined) return; // this dumb type guard is needed, because we are accessing the array via enum, and so typescript doesn't account for the above type guards. This will be solved with #842. The below array indices won't need to be hardcoded either.

req.session.levelState[3].defences = configureDefence(
defenceId,
req.session.levelState[level].defences,
req.session.levelState[3].defences,
config
);
res.send();
Expand All @@ -88,27 +139,44 @@ function handleResetSingleDefence(
) {
const defenceId = req.body.defenceId;
const configId = req.body.configId;
const level = LEVEL_NAMES.SANDBOX; //configuration only available in sandbox
if (defenceId && configId) {
req.session.levelState[level].defences = resetDefenceConfig(
defenceId,
configId,
req.session.levelState[level].defences
const level = LEVEL_NAMES.SANDBOX; //configuration only available in sandbox (interesting that we force that here, but not in the above endpoints)

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (!configId) {
sendErrorResponse(res, 400, 'Missing configId');
return;
}

if (req.session.levelState[level].defences === undefined) {
sendErrorResponse(
res,
400,
'You cannot reset defences on this level, because it uses the default defences'
);
const updatedDefenceConfig: DefenceConfigItem | undefined =
req.session.levelState[level].defences
.find((defence) => defence.id === defenceId)
?.config.find((config) => config.id === configId);

if (updatedDefenceConfig) {
res.send(updatedDefenceConfig);
} else {
res.status(400);
res.send();
}
return;
}

req.session.levelState[level].defences = resetDefenceConfig(
defenceId,
configId,
req.session.levelState[level].defences
);
const updatedDefenceConfig: DefenceConfigItem | undefined =
req.session.levelState[level].defences
.find((defence) => defence.id === defenceId)
?.config.find((config) => config.id === configId);

if (updatedDefenceConfig) {
res.send(updatedDefenceConfig);
} else {
res.status(400);
res.send();
res.send(
"something went wrong while resetting the defence's config. Check the defenceId and configId."
);
}
}

Expand Down
22 changes: 12 additions & 10 deletions backend/src/models/level.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,24 @@ enum LEVEL_NAMES {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}

function getInitialLevelStates() {
return Object.values(LEVEL_NAMES)
.filter((value) => Number.isNaN(Number(value)))
.map(
(value) =>
({
level: value as LEVEL_NAMES,
chatHistory: [],
defences: defaultDefences,
sentEmails: [],
} as LevelState)
);
.map((value) => {
return {
level: value as LEVEL_NAMES,
chatHistory: [],
defences:
value === 'LEVEL_1' || value === 'LEVEL_2'
? undefined
: defaultDefences,
sentEmails: [],
} as LevelState;
});
}

export { LEVEL_NAMES, getInitialLevelStates };
Expand Down
3 changes: 2 additions & 1 deletion backend/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ app.listen(port, () => {
console.debug('Document vector store initialized');
})
.catch((err) => {
throw new Error(`Error initializing document vectors: ${err}`);
//throw new Error(`Error initializing document vectors: ${err}`);
console.log('ERROR SWALLOWED :D', err);
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
});

Promise.all([modelsPromise, vectorsPromise])
Expand Down
2 changes: 1 addition & 1 deletion backend/test/integration/chatController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ declare module 'express-session' {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}
}
Expand Down
2 changes: 1 addition & 1 deletion backend/test/unit/controller/chatController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ declare module 'express-session' {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}
}
Expand Down
11 changes: 7 additions & 4 deletions backend/test/unit/controller/defenceController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ declare module 'express-session' {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}
}
Expand All @@ -41,7 +41,7 @@ describe('handleConfigureDefence', () => {
const req: DefenceConfigureRequest = {
body: {
defenceId: DEFENCE_ID.PROMPT_EVALUATION_LLM,
level: LEVEL_NAMES.LEVEL_1,
level: LEVEL_NAMES.SANDBOX,
config: [
{
id: 'PROMPT',
Expand All @@ -51,8 +51,11 @@ describe('handleConfigureDefence', () => {
},
session: {
levelState: [
{},
{},
{},
{
level: LEVEL_NAMES.LEVEL_1,
level: LEVEL_NAMES.SANDBOX,
chatHistory: [] as ChatMessage[],
sentEmails: [] as EmailInfo[],
defences: [] as Defence[],
Expand Down Expand Up @@ -84,7 +87,7 @@ describe('handleConfigureDefence', () => {
},
]
);
expect(req.session.levelState[LEVEL_NAMES.LEVEL_1].defences).toEqual(
expect(req.session.levelState[LEVEL_NAMES.SANDBOX].defences).toEqual(
configuredDefences
);
});
Expand Down
2 changes: 1 addition & 1 deletion backend/test/unit/controller/emailController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ declare module 'express-session' {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}
}
Expand Down
2 changes: 1 addition & 1 deletion backend/test/unit/controller/resetController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ declare module 'express-session' {
interface LevelState {
level: LEVEL_NAMES;
chatHistory: ChatMessage[];
defences: Defence[];
defences?: Defence[];
sentEmails: EmailInfo[];
}
}
Expand Down
Loading